On Tue, Jan 7, 2025 at 4:47 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > On Mon, Jan 6, 2025 at 5:37 AM Usman Akinyemi > <usmanakinyemi202@xxxxxxxxx> wrote: > > As some issues that can happen with a Git client can be operating system > > specific, it can be useful for a server to know which OS a client is > > using. In the same way it can be useful for a client to know which OS > > a server is using. > > > > Let's introduce a new protocol (`os-version`) allowing Git clients and > > servers to exchange operating system information. The protocol is > > controlled by the new `transfer.advertiseOSVersion` config option. > > > > Add the `transfer.advertiseOSVersion` config option to address > > privacy concerns issue. It defaults to `true` and can be changed to > > `false`. When enabled, this option makes clients and servers send each > > other the OS name (e.g., "Linux" or "Windows"). The information is > > retrieved using the 'sysname' field of the `uname(2)` system call. > > > > However, there are differences between `uname(1)` (command-line utility) > > and `uname(2)` (system call) outputs on Windows. These discrepancies > > complicate testing on Windows platforms. For example: > > - `uname(1)` output: MINGW64_NT-10.0-20348.3.4.10-87d57229.x86_64\ > > .2024-02-14.20:17.UTC.x86_64 > > - `uname(2)` output: Windows.10.0.20348 > > > > Until a good way to test the feature on Windows is found, the > > transfer.advertiseOSVersion is set to false on Windows during testing. > > This is because the uname(2) you mention above is not actually > system-supplied but is instead faked up Git itself for the Git for > Windows port. See git/compat/mingw.c:uname(). > > The typical way to work around this sort of issue is to ensure that > you check Git against Git itself instead of checking Git against > "system". To do so, you would implement a new "test-util" command, say > `test-util uname`, in git/t/helpers/test-uname.c which internally > calls the same uname() function that other parts of Git call. Doing so > ensures consistency of output. > > Whether or not it makes sense to go through that extra work for this > particular case is a different question. Hi Eric, Thank you for the explanation. I will look into it. > > > Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> > > Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> > > --- > > diff --git a/t/t5555-http-smart-common.sh b/t/t5555-http-smart-common.sh > > @@ -123,9 +123,19 @@ test_expect_success 'git receive-pack --advertise-refs: v1' ' > > test_expect_success 'git upload-pack --advertise-refs: v2' ' > > + printf "agent=FAKE" >agent_and_os_name && > > + if test_have_prereq WINDOWS > > + then > > + # We do not use test_config here so that any tests below can reuse > > + # the "expect" file from this test > > + git config transfer.advertiseOSVersion false > > Should this have a comment explaining why you're disabling > transfer.advertiseOSVersion, in particular that you found uname() on > Windows unreliable, thus need to disable the check for this case? > > The comment you did compose exposes a fragility of the tests: in > particular that subsequent tests rely upon a side-effect of this test. > The fact that you had to include a special comment explaining the > problem argues for a cleaner solution, such as splitting out part of > this code into a separate test which comes before this one: > specifically, a "setup"-type test which creates the "expect" file > which gets reused by multiple tests. I will work on it and update it in the next iteration. Thank you very much. Usman. > > > + else > > + printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_os_name > > + fi && > > diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh > > @@ -8,13 +8,23 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > test_expect_success 'test capability advertisement' ' > > + printf "agent=git/$(git version | cut -d" " -f3)" >agent_and_os_name && > > + if test_have_prereq WINDOWS > > + then > > + # We do not use test_config here so that tests below will be able to reuse > > + # the expect.base and expect.trailer files > > + git config transfer.advertiseOSVersion false > > Ditto. > > > + else > > + printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_os_name > > + fi &&
