snowflake: update to 2.14.1 - #30337
Conversation
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. The bump itself looks clean: PKG_RELEASE correctly stays at 1 for a version update, PKG_SOURCE_VERSION follows PKG_VERSION automatically, the new PKG_MIRROR_HASH is exercised by the build jobs (all targets built), and the commit message body matches the diff (v2.13.1..v2.14.1 upstream range) with a valid Signed-off-by. One inline note about the package's runtime test; nothing there blocks a merge on its own.
Generated by Claude Code
|
|
||
| PKG_NAME:=snowflake | ||
| PKG_VERSION:=2.13.1 | ||
| PKG_VERSION:=2.14.1 |
There was a problem hiding this comment.
The runtime test job fails on every target that runs it (x86_64, aarch64_generic, arm_cortex-a15_neon-vfpv4, i386_pentium-mmx, mips_24kc — same output in each) because two of the five binaries never print the version string:
snowflake-broker: [warn] Version check (/usr/bin/snowflake-broker)
flag provided but not defined: -?
Usage of /usr/bin/snowflake-broker:
snowflake-broker: No executables in the package provided version 2.14.1
snowflake-broker: Generic tests failed
snowflake-probetest fails the same way; client, proxy and server all print <name> 2.14.1 and pass. This looks like it predates the bump rather than being caused by it — broker and probetest have no version flag upstream — but it does keep the job red on every push to this package, and net/snowflake/ currently has no test script. Per the feed's testing rules, a test-version.sh in net/snowflake/ that returns 0 for those two package names is the documented way to bypass the forced generic version check (and note grep -q should be avoided in it, so the checked strings stay visible in CI logs). Worth folding in here or in a follow-up, your call.
Generated by Claude Code
7263c39 to
2419880
Compare
Upstream commits v2.13.1..v2.14.1: 77db339 Update covert-dtls and improve public API cc1fbc8 Add none string flag to covert-dtls config 3003cc9 Broker: Rely on KnownProxyTypes for metrics ff32ce4 Add bloco to map of KnownProxyTypes 4204e26 Proxy: Update when EventOnCurrentNATTypeDetermined fires 22520bd Broker: Represent pollInterval in ms as time.Duration ce719c3 Broker: Load poll intervals from file ffb3b00 Broker: Fix potential nil-pointer dereference 96560c8 Report listen error in Metrics.Start. cee56c1 Ensure self-reported client counts are 0 or greater 2687e20 Proxy: Refactor pollOffer to return ProxyPollResponse a555c3d Proxy: Add poll loop ticker to SnowflakeProxy struct 28224d0 Proxy: Return immediately from runSession after poll 0d925e7 Proxy: Adjust poll ticker according to ProxyPollResponse 7c9cf13 Bump version of Snowflake to 2.14.0 38a81ff add socks5 based interactive connectivity test 4b8818e probetest: improve local candidcate removal 7639a35 probetest: better error handling and shortened variable name 1f922de probetest: update README 1bb418c Common: Add failing test to deserialize bad offer b9b3088 Common: Check type assertions when deserializing SDP 3941c8c Broker: Validate WebRTC offers and answers cd33fc6 Bump version of Snowflake to v2.14.1 Added test.sh: CI's forced generic runtime test fails on every target for two of the five binaries, snowflake-broker and snowflake-probetest, because neither implements a version flag - confirmed directly against the 2.14.1 source, where client/proxy/server all define `-version` (flag.Bool("version", ...)) but broker and probetest have no such flag at all, so the harness's probe falls through to "flag provided but not defined" and the generic check reports no version string found. This predates the 2.14.1 bump, not caused by it. Rather than matching per-binary version strings, test.sh checks that each of the five actually runs: `-h` is handled by Go's flag package for all of them (none override it away), and every one prints some form of usage text in response - the default "Usage of <path>:" for broker/client/probetest/proxy, or server's own "Usage: <name> [OPTIONS]" since it installs a custom flag.Usage. Both forms share the substring "Usage", so one check covers all five without depending on $PKG_VERSION. Verified against native builds of all five binaries from the same 2.14.1 source. Signed-off-by: Daniel Golle <[email protected]>
2419880 to
3ee6b83
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit (force-push of snowflake: update to 2.14.1, adding net/snowflake/test.sh).
Commit checks
- 3ee6b83 "snowflake: update to 2.14.1" — the bump part matches the diff (
PKG_RELEASEcorrectly stays at1for a version update,PKG_SOURCE_VERSIONfollowsPKG_VERSION, and the newPKG_MIRROR_HASHis exercised by the build jobs, which fetched and built on all targets). The added paragraphs abouttest.sh, however, present it as the response to the forced generic version check failing forsnowflake-brokerandsnowflake-probetest; the harness runs the generic block regardless oftest.sh, so the runtime job is still failing on the same five targets at this head. Details inline onnet/snowflake/test.sh.
Nothing here is a packaging defect in the bump itself — the only open item is the still-red runtime job, which needs a test-version.sh in addition to the new test.sh. The mixed-scope note is a nit and does not block.
Generated by Claude Code
| # broker and probetest have no -version flag upstream, so this | ||
| # checks that each binary runs and prints its usage rather than | ||
| # matching a version/identity string | ||
| "$1" -h 2>&1 | grep "Usage" |
There was a problem hiding this comment.
This test.sh runs and passes, but it does not take the place of the generic version check, so the runtime job is still red at this head on aarch64_generic, arm_cortex-a15_neon-vfpv4, i386_pentium-mmx, mips_24kc and x86_64 (identical output on each):
snowflake-broker: [skip] Version check override
snowflake-broker: No executables in the package provided version 2.14.1
snowflake-broker: Generic tests failed
snowflake-broker: Use the package-specific test.sh
Usage of snowflake-broker:
snowflake-broker: Test passed
The container is started with -e FORCE_GENERIC_TESTS=true, so the generic block runs for every package independently of test.sh, and Generic tests failed on its own makes the job exit 1 (Some tests failed). snowflake-probetest produces the same three lines; client/proxy/server pass. The [skip] Version check override line is the harness looking for a test-version.sh and not finding one — that script is the hook that suppresses the version check, test.sh is additive to it rather than a replacement.
So a net/snowflake/test-version.sh exiting 0 for snowflake-broker and snowflake-probetest (with the reason in a comment, since neither defines a version flag upstream) is still needed to turn the job green; this test.sh can stay as the extra behavioural check. lang/perl/test-version.sh#L6-L9 is the same shape. Note the grep "Usage" here already avoids quiet mode, which is what the feed's testing rules ask for.
Generated by Claude Code
| @@ -0,0 +1,14 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
nit: mixed scope — the commit subject is snowflake: update to 2.14.1, but roughly two thirds of the commit body is about this new file, which is unrelated to the version bump (the broker/probetest version-flag gap predates 2.14.1, as the body itself notes). Splitting into snowflake: update to 2.14.1 plus a separate snowflake: add runtime test script would keep the bump revertable on its own. The PR description also only describes the version bump and does not mention the test script at all.
Generated by Claude Code
📦 Package Details
Maintainer: @dangowrt
(You can find this by checking the history of the package
Makefile.)Description:
Git-sourced rolling package, from v2.13.1. Notable upstream changes: broker WebRTC offer/answer validation, SDP deserialization type-assertion checks, proxy poll-loop refactor (ticker, immediate return from runSession, adjust per ProxyPollResponse), broker poll-interval-from-file loading and nil-pointer-dereference fix, and covert-dtls config/API improvements. Full commit range (v2.13.1..v2.14.1) is in the commit message.
🧪 Run Testing Details
✅ Formalities
If your PR contains a patch:
git am(e.g., subject line, commit description, etc.)
We must try to upstream patches to reduce maintenance burden.