test: NetBSD support
cover test: NetBSD support
Likely still missing changes as it only completes a run with a minimal number of dependencies but open for feedback
Requires pkgsrc packages for gmake, perl, bash and curl and completes a run
$ gmake SHELL_PATH=/usr/pkg/bin/bash NO_PYTHON=1 CURL_DIR=/usr/pkg test
Carlo Marcelo Arenas Belón (7):
Documentation: update INSTALL for NetBSD
t0301: remove trailing / for dir creation config.mak.uname: NetBSD uses BSD semantics with fread for directories config.mak.uname: NetBSD uses old iconv interface
test-lib: use pkgsrc provided unzip for NetBSD
t5004: use GNU tar to avoid known issues with BSD tar config.mak.uname: use pkgsrc perl for NetBSD
INSTALL | 6 +++--- config.mak.uname | 3 +++ t/t0301-credential-cache.sh | 8 ++++---- t/t5004-archive-corner-cases.sh | 2 ++ t/test-lib.sh | 1 + 5 files changed, 13 insertions(+), 7 deletions(-)
Signature · 1 line
2.20.0.rc1
RFC PATCH 1/7 Documentation: update INSTALL for NetBSD
NetBSD added a BSD licensed reimplementation of GNU libintl to its base at least since release 4.0 (mid 2012) and git can be configured to build with it.
--- INSTALL | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
INSTALL +3 -3
@@ -154,11 +154,11 @@ Issues of note: git-gui, you can use NO_TCLTK. - A gettext library is used by default for localizing Git. The - primary target is GNU libintl, but the Solaris gettext - implementation also works. + primary target is GNU libintl, but the Solaris and NetBSD gettext + implementations also work. We need a gettext.h on the system for C code, gettext.sh (or - Solaris gettext(1)) for shell scripts, and libintl-perl for Perl + gettext(1) utility) for shell scripts, and libintl-perl for Perl programs. Set NO_GETTEXT to disable localization support and make Git only
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 1/7] Documentation: update INSTALL for NetBSD
Quoted 8 lines · Carlo Marcelo Arenas Belón
NetBSD added a BSD licensed reimplementation of GNU libintl to its base at least since release 4.0 (mid 2012) and git can be configured to build with it.
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>--- INSTALL | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Looks good.
Quoted 19 lines
INSTALL +3
-3@@ -154,11 +154,11 @@ Issues of note: git-gui, you can use NO_TCLTK. - A gettext library is used by default for localizing Git. The - primary target is GNU libintl, but the Solaris gettext - implementation also works. + primary target is GNU libintl, but the Solaris and NetBSD gettext + implementations also work. We need a gettext.h on the system for C code, gettext.sh (or - Solaris gettext(1)) for shell scripts, and libintl-perl for Perl + gettext(1) utility) for shell scripts, and libintl-perl for Perl programs. Set NO_GETTEXT to disable localization support and make Git only
RFC PATCH 2/7 t0301: remove trailing / for dir creation
In reply to Carlo Marcelo Arenas Belónthe semantics of how mkdir -p should work, specially when using -m are not standard and in this case NetBSD will assume that the permision should not be changed, breaking the test
-p is technically not needed either, but will be cleared in a future patch eventhough it could be considered an alternative fix
--- t/t0301-credential-cache.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
t/t0301-credential-cache.sh +4 -4
@@ -77,9 +77,9 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' test_expect_success 'use user socket if user directory exists' ' test_when_finished " git credential-cache exit && - rmdir \"\$HOME/.git-credential-cache/\" + rmdir \"\$HOME/.git-credential-cache\" " && - mkdir -p -m 700 "$HOME/.git-credential-cache/" && + mkdir -p -m 700 "$HOME/.git-credential-cache" && check approve cache <<-\EOF && protocol=https host=example.com
@@ -92,10 +92,10 @@ test_expect_success 'use user socket if user directory exists' ' test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' ' test_when_finished " git credential-cache exit && - rmdir \"\$HOME/dir/\" && + rmdir \"\$HOME/dir\" && rm \"\$HOME/.git-credential-cache\" " && - mkdir -p -m 700 "$HOME/dir/" && + mkdir -p -m 700 "$HOME/dir" && ln -s "$HOME/dir" "$HOME/.git-credential-cache" && check approve cache <<-\EOF && protocol=https
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 2/7] t0301: remove trailing / for dir creation
Quoted 3 lines · Carlo Marcelo Arenas Belón
the semantics of how mkdir -p should work, specially when using -m are not standard and in this case NetBSD will assume that the permision should not be changed, breaking the test
This does not explain, except for the fuzzy "in this case", why we want to lose trailing slash at all. If what you wanted to say was
On NetBSD, "mkdir -p -m $bits path/to/dir/" ignores the permission bits when creating the directory component 'dir', but without the trailing slash at the end of "dir/", it works as expected.
then that would be an understandable justification for the patch. If you meant to say something else, then I couldn't read it from what you wrote, so the log message needs updating to help future readers.
Quoted 2 lines
-p is technically not needed either, but will be cleared in a future patch eventhough it could be considered an alternative fix
I haven't seen the steps 3-7/7 yet, but if they remove "-p", then "but" would be a strange thing to say here. If they indeed do, then perhaps:
The '-p' option is not needed in thse cases, as we know $HOME/ exists in the test environment and we are creating a new directory directly under it. It will be removed in a future patch in the series. Removing '-p' could be an alternative fix, as the command then works as expected even on NetBSD with the trailing slash after directory name.
On the other hand, if future changes make it necessary to create two or more levels here, then
At this point in the series, '-p' is not needed in thse cases, as we know $HOME/ exists in the test environment and we are creating .git-credential-cache directory directly under it. However, in later patches, we'll make it necessary for these 'mkdir -p' to create two or more levels, so removing '-p' would be an alternative fix for this step, but would not work for the series as a whole.
would also make sense. I simply do not have enough information yet to tell which at this point in the series.
Quoted 34 lines
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>--- t/t0301-credential-cache.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)t/t0301-credential-cache.sh +4
-4@@ -77,9 +77,9 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' test_expect_success 'use user socket if user directory exists' ' test_when_finished " git credential-cache exit && - rmdir \"\$HOME/.git-credential-cache/\" + rmdir \"\$HOME/.git-credential-cache\" " && - mkdir -p -m 700 "$HOME/.git-credential-cache/" && + mkdir -p -m 700 "$HOME/.git-credential-cache" && check approve cache <<-\EOF && protocol=https host=example.com@@ -92,10 +92,10 @@ test_expect_success 'use user socket if user directory exists' ' test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' ' test_when_finished " git credential-cache exit && - rmdir \"\$HOME/dir/\" && + rmdir \"\$HOME/dir\" && rm \"\$HOME/.git-credential-cache\" " && - mkdir -p -m 700 "$HOME/dir/" && + mkdir -p -m 700 "$HOME/dir" && ln -s "$HOME/dir" "$HOME/.git-credential-cache" && check approve cache <<-\EOF && protocol=https
RFC PATCH 3/7 config.mak.uname: NetBSD uses BSD semantics with fread for directories
In reply to Carlo Marcelo Arenas Belónthis "fixes" test 23 (proper error on directory "files") from t1308
other BSD (OpenBSD, MirBSD) likely also affected but they will be fixed in a different series
the optional 'configure' sets this automatically and is probably what most users from this platform had been doing as a workaround
--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)
config.mak.uname +1 -0
@@ -253,6 +253,7 @@ ifeq ($(uname_S),NetBSD) HAVE_BSD_SYSCTL = YesPlease HAVE_BSD_KERN_PROC_SYSCTL = YesPlease PROCFS_EXECUTABLE_PATH = /proc/curproc/exe + FREAD_READS_DIRECTORIES = UnfortunatelyYes endif ifeq ($(uname_S),AIX) DEFAULT_PAGER = more
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 3/7] config.mak.uname: NetBSD uses BSD semantics with fread for directories
Quoted 7 lines · Carlo Marcelo Arenas Belón
this "fixes" test 23 (proper error on directory "files") from t1308
other BSD (OpenBSD, MirBSD) likely also affected but they will be fixed in a different series
the optional 'configure' sets this automatically and is probably what most users from this platform had been doing as a workaround
Yup, thanks for straightening this out.
Quoted 18 lines
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)config.mak.uname +1
-0@@ -253,6 +253,7 @@ ifeq ($(uname_S),NetBSD) HAVE_BSD_SYSCTL = YesPlease HAVE_BSD_KERN_PROC_SYSCTL = YesPlease PROCFS_EXECUTABLE_PATH = /proc/curproc/exe + FREAD_READS_DIRECTORIES = UnfortunatelyYes endif ifeq ($(uname_S),AIX) DEFAULT_PAGER = more
RFC PATCH 4/7 config.mak.uname: NetBSD uses old iconv interface
In reply to Carlo Marcelo Arenas Belónprevents the following warning :
utf8.c: In function 'reencode_string_iconv':
utf8.c:486:28: warning: passing argument 2 of 'iconv' from incompatible pointer type [-Wincompatible-pointer-types]
size_t cnt = iconv(conv, &cp, &insz, &outpos, &outsz);
^
In file included from git-compat-util.h:275:0,
from utf8.c:1:
/usr/include/iconv.h:46:8: note: expected 'const char ** restrict' but argument is of type 'char **'
size_t iconv(iconv_t, const char ** __restrict,
^it is set by optional configure at least since NetBSD 6.0
--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)
config.mak.uname +1 -0
@@ -246,6 +246,7 @@ ifeq ($(uname_S),NetBSD) ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2) NEEDS_LIBICONV = YesPlease endif + OLD_ICONV = UnfortunatelyYes BASIC_CFLAGS += -I/usr/pkg/include BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib USE_ST_TIMESPEC = YesPlease
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 4/7] config.mak.uname: NetBSD uses old iconv interface
Quoted 1 line · Carlo Marcelo Arenas Belón
prevents the following warning :
s/^/Doing so / or something to make it a complete sentence.
Quoted 2 lines
...
it is set by optional configure at least since NetBSD 6.0
s/it/It/;
Again, makes sense, and thanks for tying this loose end.
Quoted 17 lines
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)config.mak.uname +1
-0@@ -246,6 +246,7 @@ ifeq ($(uname_S),NetBSD) ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2) NEEDS_LIBICONV = YesPlease endif + OLD_ICONV = UnfortunatelyYes BASIC_CFLAGS += -I/usr/pkg/include BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib USE_ST_TIMESPEC = YesPlease
RFC PATCH 5/7 test-lib: use pkgsrc provided unzip for NetBSD
In reply to Carlo Marcelo Arenas Belónd98b2c5fce ("test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/", 2016-07-21) added an exception to the test suite for FreeBSD because the tests assume functionality not provided by its base unzip tool.
NetBSD shares that limitation and provides a package that could be used instead so all tests from t5003 succeed
--- t/test-lib.sh | 1 + 1 file changed, 1 insertion(+)
t/test-lib.sh +1 -0
@@ -1244,6 +1244,7 @@ test_lazy_prereq SANITY ' ' test FreeBSD != $uname_s || GIT_UNZIP=${GIT_UNZIP:-/usr/local/bin/unzip} +test $uname_s = NetBSD && GIT_UNZIP=${GIT_UNZIP:-/usr/pkg/bin/unzip} GIT_UNZIP=${GIT_UNZIP:-unzip} test_lazy_prereq UNZIP ' "$GIT_UNZIP" -v
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 5/7] test-lib: use pkgsrc provided unzip for NetBSD
Quoted 17 lines · Carlo Marcelo Arenas Belón
d98b2c5fce("test-lib: on FreeBSD, look for unzip(1) in/usr/local/bin/", 2016-07-21) added an exception to the test suite for FreeBSD because the tests assume functionality not provided by its base unzip tool.NetBSD shares that limitation and provides a package that could be used instead so all tests from t5003 succeed
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>--- t/test-lib.sh | 1 + 1 file changed, 1 insertion(+)t/test-lib.sh +0
-0
t/test-lib.sh test_lazy_prereq SANITY ' · line 1247
Quoted 4 lines
t/test-lib.sh +1
-0@@ -1244,6 +1244,7 @@ test_lazy_prereq SANITY ' ' test FreeBSD != $uname_s || GIT_UNZIP=${GIT_UNZIP:-/usr/local/bin/unzip} +test $uname_s = NetBSD && GIT_UNZIP=${GIT_UNZIP:-/usr/pkg/bin/unzip}
This is OK for now, but I wonder why this is done in test-lib.sh in the first place, unlike $(TAR) that is set and configurable from the top level. The difference is that GIT_UNZIP happens to be used only in tests, while TAR is used in the primary build procedure.
But I suspect that in the longer run, we should allow UNZIP to be given next to TAR to the top-level Makefile and export it down. That would allow the usual mechanism like config.mak.uname, ./configure and "make TAR=... UNZIP=..." command line override to be used uniformly, without people having to worry about the distinction. The builders should *not* have to care that one is used in the build and the other is only used in the test.
Quoted 3 lines
t/test-lib.sh +0
-0GIT_UNZIP=${GIT_UNZIP:-unzip} test_lazy_prereq UNZIP ' "$GIT_UNZIP" -v
RFC PATCH 6/7 t5004: use GNU tar to avoid known issues with BSD tar
In reply to Carlo Marcelo Arenas Belón56ee96572a ("t5004: resurrect original empty tar archive test", 2013-05-09) added a test to try to detect and workaround issues with the standard tar from BSD, but at least in NetBSD would be better to instead require GNU tar which is available from pkgsrc
--- t/t5004-archive-corner-cases.sh | 2 ++ 1 file changed, 2 insertions(+)
t/t5004-archive-corner-cases.sh +2 -0
@@ -31,6 +31,8 @@ test_lazy_prereq UNZIP_ZIP64_SUPPORT ' "$GIT_UNZIP" -v | grep ZIP64_SUPPORT ' +test $uname_s = NetBSD && TAR="gtar" + # bsdtar/libarchive versions before 3.1.3 consider a tar file with a # global pax header that is not followed by a file record as corrupt. if "$TAR" tf "$TEST_DIRECTORY"/t5004/empty-with-pax-header.tar >/dev/null 2>&1
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 6/7] t5004: use GNU tar to avoid known issues with BSD tar
Quoted 15 lines · Carlo Marcelo Arenas Belón
56ee96572a("t5004: resurrect original empty tar archive test", 2013-05-09) added a test to try to detect and workaround issues with the standard tar from BSD, but at least in NetBSD would be better to instead require GNU tar which is available from pkgsrcSigned-off-by Carlo Marcelo Arenas Belón <[email protected]>--- t/t5004-archive-corner-cases.sh | 2 ++ 1 file changed, 2 insertions(+)t/t5004-archive-corner-cases.sh +0
-0
t/t5004-archive-corner-cases.sh test_lazy_prereq UNZIP_ZIP64_SUPPORT ' · line 35
Quoted 5 lines
t/t5004-archive-corner-cases.sh +2
-0@@ -31,6 +31,8 @@ test_lazy_prereq UNZIP_ZIP64_SUPPORT ' "$GIT_UNZIP" -v | grep ZIP64_SUPPORT ' +test $uname_s = NetBSD && TAR="gtar" +
This smells wrong.
Isn't the top-level Makefile ask you to use TAR=gtar if that is the usable implementation of tar on your platform, and isn't what you specify as $(TAR) exported down to the t/Makefile to be used here?
Quoted 3 lines
t/t5004-archive-corner-cases.sh +0
-0# bsdtar/libarchive versions before 3.1.3 consider a tar file with a # global pax header that is not followed by a file record as corrupt. if "$TAR" tf "$TEST_DIRECTORY"/t5004/empty-with-pax-header.tar >/dev/null 2>&1
RFC PATCH 7/7 config.mak.uname: use pkgsrc perl for NetBSD
In reply to Carlo Marcelo Arenas Belónotherwise will default to /usr/bin/perl which wouldn't normally exist
--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)
config.mak.uname +1 -0
@@ -249,6 +249,7 @@ ifeq ($(uname_S),NetBSD) OLD_ICONV = UnfortunatelyYes BASIC_CFLAGS += -I/usr/pkg/include BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib + PERL_PATH = /usr/pkg/bin/perl USE_ST_TIMESPEC = YesPlease HAVE_PATHS_H = YesPlease HAVE_BSD_SYSCTL = YesPlease
Signature · 1 line
2.20.0.rc1
Re: [RFC PATCH 7/7] config.mak.uname: use pkgsrc perl for NetBSD
Quoted 6 lines · Carlo Marcelo Arenas Belón
otherwise will default to
/usr/bin/perlwhich wouldn't normally existSigned-off-by Carlo Marcelo Arenas Belón <[email protected]>--- config.mak.uname | 1 + 1 file changed, 1 insertion(+)
I do not have experience with NetBSD so I take your words on face value. This patch makes sense to me.
Thanks.
Quoted 12 lines
config.mak.uname +1
-0@@ -249,6 +249,7 @@ ifeq ($(uname_S),NetBSD) OLD_ICONV = UnfortunatelyYes BASIC_CFLAGS += -I/usr/pkg/include BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib + PERL_PATH = /usr/pkg/bin/perl USE_ST_TIMESPEC = YesPlease HAVE_PATHS_H = YesPlease HAVE_BSD_SYSCTL = YesPlease
Quoted 6 lines · Carlo Marcelo Arenas Belón
Likely still missing changes as it only completes a run with a minimal number of dependencies but open for feedback
Requires pkgsrc packages for gmake, perl, bash and curl and completes a run
$ gmake SHELL_PATH=/usr/pkg/bin/bash NO_PYTHON=1 CURL_DIR=/usr/pkg test
This looks good to me, and fixes most of the issues I've encountered on NetBSD recently. See
https://gitlab.com/git-vcs/git-gitlab-ci/blob/9337ed6f99e3780d1d8dc087dff688f5a68805a4/ci/gitlab/run-on-gcc-farm.sh#L228-239
and https://gitlab.com/git-vcs/git-ci/-/jobs/125476939