Yicong-Huang commented on code in PR #7141:
URL: https://github.com/apache/texera/pull/7141#discussion_r3786758763
##########
bin/local-dev/main.sh:
##########
@@ -647,8 +715,9 @@ _detect_host_lan_ip() {
case "$(uname -s 2>/dev/null)" in
Darwin) _detect_host_lan_ip_darwin ;;
Linux) _detect_host_lan_ip_linux ;;
- # Anything else (BSD, WSL oddities): try both rather than give up.
- *) _detect_host_lan_ip_darwin || _detect_host_lan_ip_linux ;;
+ MINGW*|MSYS*|CYGWIN*|*_NT*) _detect_host_lan_ip_windows ;;
Review Comment:
This arm makes the failure message wrong. Git Bash reports
`MINGW64_NT-10.0-…`, which falls to the `*)` case in `_require_host_lan_ip`
(`main.sh:736`). So a Windows user whose detection fails is told that none of
"the macOS and Linux probes" found an address, and pointed at `route get
default` and `ip route show default`. Neither exists on their machine; neither
ran.
Give `_require_host_lan_ip` the same `MINGW*|MSYS*|CYGWIN*|*_NT*` arm you
added here, naming the two netsh commands instead. Raised last round in the
review body, which had no anchor to hang it on; here is one.
##########
bin/local-dev/main.sh:
##########
@@ -593,6 +593,74 @@ fi
# Both platform probes follow the same two steps: the interface backing the
# default route first (most reliable on a laptop that may have wifi +
# thunderbolt + tailscale all active), then a scan as a fallback.
+_detect_host_lan_ip_windows() {
+ local iface_details="" local_ip="" iface_name="" idx=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|tailscale|zerotier|wg|McAfee"
+ # Use 'netsh interface ip show route' to find the list of interfaces
+ # associated with the 0.0.0.0/0 (default) route and trace the respective
indices
+ local idx_list
+ idx_list=$(netsh interface ip show route 2>/dev/null | \
+ awk '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ print $(i+1)
+ }
+ }
+ }')
+
+ # Iterate through candidate indices, inspect adapter name and grab the
first physical LAN IP
+ for idx in $idx_list; do
+ iface_details=$(netsh interface ip show addresses "$idx" 2>/dev/null)
+
+ # Skip if adapter name is empty or matches virtual/VPN exclusions
+ iface_name=$(echo "$iface_details" | awk -F'"' '/Configuration for
interface/ {print $2}')
+ if [[ -z "$iface_name" ]] || echo "$iface_name" | grep -qiE
"$virt_excl"; then
+ continue
+ fi
+
+ # Get the IPv4 address of the interface
+ local_ip=$(echo "$iface_details" | awk -F': ' '/IP Address/ {print $2;
exit}' | tr -d ' \r')
+ # Validate non-loopback and non-APIPA (169.254.x.x) address
+ if [[ -n "$local_ip" && "$local_ip" != 127.* && "$local_ip" !=
169.254.* ]]; then
+ printf '%s\n' "$local_ip"
+ return 0
+ fi
+ done
+
+ # FALLBACK METHOD
+ # Parse all netsh address blocks, filter out virtual adapters, and pick
the first valid LAN IP
+ local_ip=$(
+ netsh interface ip show addresses 2>/dev/null |
+ awk -v excl="$virt_excl" '
+ /Configuration for interface/ {
+ split($0, a, "\"")
+ dev = a[2]
+ valid_iface = (dev !~ excl)
Review Comment:
awk's `~` doesn't fold case, so this drops the nine lowercase tokens `:618`
catches with `grep -qiE` — `tailscale`, `zerotier`, `tap`, `tun`, `cni`,
`flannel`, `cali`, `kube`, `wg`. It is last round's `:491` finding, fixed in
step 1 and missed by the step 2 written beside it.
Worse than a plain miss, because this stage runs *after* step 1 refused the
adapter. On a full-tunnel host whose only default route is `Tailscale`, step 1
skips it and this scan returns it — so `HOST_LAN_IP` becomes the one address
lakekeeper cannot reach. Reproduced; `ZeroTier One [...]` behaves the same.
```suggestion
valid_iface = (tolower(dev) !~ tolower(excl))
```
##########
bin/local-dev/main.sh:
##########
@@ -593,6 +593,74 @@ fi
# Both platform probes follow the same two steps: the interface backing the
# default route first (most reliable on a laptop that may have wifi +
# thunderbolt + tailscale all active), then a scan as a fallback.
+_detect_host_lan_ip_windows() {
+ local iface_details="" local_ip="" iface_name="" idx=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|tailscale|zerotier|wg|McAfee"
+ # Use 'netsh interface ip show route' to find the list of interfaces
+ # associated with the 0.0.0.0/0 (default) route and trace the respective
indices
+ local idx_list
+ idx_list=$(netsh interface ip show route 2>/dev/null | \
+ awk '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ print $(i+1)
+ }
+ }
+ }')
+
+ # Iterate through candidate indices, inspect adapter name and grab the
first physical LAN IP
+ for idx in $idx_list; do
+ iface_details=$(netsh interface ip show addresses "$idx" 2>/dev/null)
+
+ # Skip if adapter name is empty or matches virtual/VPN exclusions
+ iface_name=$(echo "$iface_details" | awk -F'"' '/Configuration for
interface/ {print $2}')
+ if [[ -z "$iface_name" ]] || echo "$iface_name" | grep -qiE
"$virt_excl"; then
+ continue
+ fi
+
Review Comment:
Whitespace-only line — still here. The thread on this was resolved without
the fix landing, and has since gone outdated, so re-anchoring at the live line.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
