Yicong-Huang commented on code in PR #7141:
URL: https://github.com/apache/texera/pull/7141#discussion_r3764514422
##########
bin/local-dev/main.sh:
##########
@@ -466,6 +466,40 @@ 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 idx="" iface_details="" local_ip=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|Local|tailscale|zerotier|wg|McAfee"
+
+ # Use 'netsh interface ip show route' to find the interface
+ # associated with the 0.0.0.0/0 (default) route.
+ idx=$(netsh interface ip show route 2>/dev/null | \
+ awk -v excl="$virt_excl" '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ # Check if Gateway/Interface name (i+2) matches
virtual pattern
+ if ($(i+2) !~ excl) {
+ print $(i+1)
+ exit
+ }
+ }
+ }
+ }' | head -n 1)
+
+ if [ -n "$idx" ]; then
+ # Query the interface configuration using the Idx value
+ iface_details=$(netsh interface ip show addresses "$idx" 2>/dev/null)
+ local_ip=$(echo "$iface_details" | awk -F': ' '/IP Address/ {print
$2}' | tr -d ' \r')
Review Comment:
An adapter with a second IPv4 emits two lines here. `tr -d ' \r'` strips
spaces and CRs but not newlines, so `HOST_LAN_IP` — and `STORAGE_S3_ENDPOINT`
at `main.sh:588` — carries a raw newline inside the URL. That control character
is verbatim the parse error #7138 reports. Both peers are structurally
single-valued: `ipconfig getifaddr` returns one address, and the linux probe
`exit`s on its first match (`main.sh:533`).
```suggestion
local_ip=$(echo "$iface_details" | awk -F': ' '/IP Address/ {print
$2; exit}' | tr -d ' \r')
```
##########
bin/local-dev/main.sh:
##########
@@ -466,6 +466,40 @@ 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 idx="" iface_details="" local_ip=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|Local|tailscale|zerotier|wg|McAfee"
+
+ # Use 'netsh interface ip show route' to find the interface
+ # associated with the 0.0.0.0/0 (default) route.
+ idx=$(netsh interface ip show route 2>/dev/null | \
+ awk -v excl="$virt_excl" '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ # Check if Gateway/Interface name (i+2) matches
virtual pattern
+ if ($(i+2) !~ excl) {
Review Comment:
The exclusion can't fire here. For a `0.0.0.0/0` row netsh puts the next-hop
**gateway IP** in the Gateway/Interface-Name column; an interface *name*
appears there only for on-link routes. A regex of adapter names never matches
an IP. Reproduced: given default routes via Idx 22 (`10.8.0.1`) and Idx 14
(`192.168.1.1`), this awk returns 22. A full-tunnel VPN's adapter therefore
wins, and `HOST_LAN_IP` becomes an address lakekeeper can't route back to —
#7138's symptom, silently.
You already run `show addresses "$idx"` below, and its output opens with
`Configuration for interface "<name>"`. Filter on that name instead, matching
what the linux probe filters (`main.sh:527-530`). Drop `Local` while moving it
— it matches the physical "Local Area Connection".
##########
bin/local-dev/main.sh:
##########
@@ -466,6 +466,40 @@ 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 idx="" iface_details="" local_ip=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|Local|tailscale|zerotier|wg|McAfee"
+
+ # Use 'netsh interface ip show route' to find the interface
+ # associated with the 0.0.0.0/0 (default) route.
+ idx=$(netsh interface ip show route 2>/dev/null | \
+ awk -v excl="$virt_excl" '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ # Check if Gateway/Interface name (i+2) matches
virtual pattern
+ if ($(i+2) !~ excl) {
+ print $(i+1)
+ exit
+ }
+ }
+ }
+ }' | head -n 1)
+
+ if [ -n "$idx" ]; then
+ # Query the interface configuration using the Idx value
+ iface_details=$(netsh interface ip show addresses "$idx" 2>/dev/null)
+ local_ip=$(echo "$iface_details" | awk -F': ' '/IP Address/ {print
$2}' | tr -d ' \r')
+
+ # Check if a valid non-loopback / non-APIPA IP was found
+ if [[ -n "$local_ip" && "$local_ip" != 127.* && "$local_ip" !=
169.254.* ]]; then
+ printf '%s\n' "$local_ip"
+ return 0
+ fi
+ fi
+ return 1
Review Comment:
No step-2 fallback here, unlike both peers: darwin scans en0-en10
(`main.sh:510-513`), linux scans every global IPv4 (`main.sh:536-549`). A
Windows host with no default route, or with netsh output this awk can't parse,
hard-exits at `main.sh:583` instead of finding what a scan would.
Either way `main.sh:466-468` needs a rewrite — it promises "the same two
steps ... then a scan as a fallback" and now sits directly above a one-step
probe. Adding the scan is the better fix: it makes that comment true again.
##########
bin/local-dev/main.sh:
##########
@@ -466,6 +466,40 @@ 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 idx="" iface_details="" local_ip=""
+
+ local
virt_excl="vEthernet|WSL|Hyper-V|VirtualBox|Docker|Bridge|tap|tun|cni|flannel|cali|kube|Local|tailscale|zerotier|wg|McAfee"
+
+ # Use 'netsh interface ip show route' to find the interface
+ # associated with the 0.0.0.0/0 (default) route.
+ idx=$(netsh interface ip show route 2>/dev/null | \
+ awk -v excl="$virt_excl" '{
+ for (i = 1; i < NF; i++) {
+ if ($i == "0.0.0.0/0") {
+ # Check if Gateway/Interface name (i+2) matches
virtual pattern
+ if ($(i+2) !~ excl) {
+ print $(i+1)
+ exit
+ }
+ }
+ }
+ }' | head -n 1)
Review Comment:
The awk `exit`s after its first print, so this stage can never see a second
line.
```suggestion
}')
```
##########
bin/local-dev/main.sh:
##########
@@ -466,6 +466,40 @@ 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() {
Review Comment:
This probe needs the test its sibling got. Test 29
(`bin/local-dev/tests/test_local_dev_sh.sh:600-690`) extracts
`_detect_host_lan_ip_linux` with awk and drives it against a fake `ip` on
`PATH` — 8 cases, one being "default route over a bridge/VPN is skipped, not
trusted".
It stubs the *tool*, so it runs anywhere. A fake `netsh` echoing canned
`show route` / `show addresses` output would cover this function on the
existing ubuntu and macos `build / infra` jobs. Both other findings here are
cases such a test pins directly.
##########
bin/local-dev/main.sh:
##########
@@ -520,8 +554,9 @@ _detect_host_lan_ip() {
case "$(uname -s 2>/dev/null)" in
Darwin) _detect_host_lan_ip_darwin ;;
Linux) _detect_host_lan_ip_linux ;;
+ MINGW*|MSYS*|CYGWIN*|*_NT*) _detect_host_lan_ip_windows ;;
# Anything else (BSD, WSL oddities): try both rather than give up.
Review Comment:
Three probes are chained on the next line now.
```suggestion
# Anything else (BSD, WSL oddities): try all three rather than give
up.
```
--
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]
