Message ID | 20210907110841.3341786-1-alxndr@bu.edu (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fuzz: fix unbound variable in build.sh | expand |
On Tuesday, 2021-09-07 at 07:08:41 -04, Alexander Bulekov wrote: > /src/build.sh: line 76: GITLAB_CI: unbound variable > Fix that. > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > --- > > This change is in preparation to revert: > 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz. > Reverting as-is produces an unbound variable complaint when we try to > build the fuzzers in the OSS-Fuzz container. > > scripts/oss-fuzz/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh > index 98b56e0521..5ddc769c9c 100755 > --- a/scripts/oss-fuzz/build.sh > +++ b/scripts/oss-fuzz/build.sh > @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then > "\nFor example: CC=clang CXX=clang++ $0" > fi > > -if [ "$GITLAB_CI" != "true" ]; then > +if [ -z ${GITLAB_CI+x} ]; then > for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do > cp "$i" "$DEST_DIR/lib/" > done > -- > 2.30.2
On 07/09/2021 13.08, Alexander Bulekov wrote: > /src/build.sh: line 76: GITLAB_CI: unbound variable > Fix that. > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > --- > > This change is in preparation to revert: > 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz. > Reverting as-is produces an unbound variable complaint when we try to > build the fuzzers in the OSS-Fuzz container. > > scripts/oss-fuzz/build.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh > index 98b56e0521..5ddc769c9c 100755 > --- a/scripts/oss-fuzz/build.sh > +++ b/scripts/oss-fuzz/build.sh > @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then > "\nFor example: CC=clang CXX=clang++ $0" > fi > > -if [ "$GITLAB_CI" != "true" ]; then > +if [ -z ${GITLAB_CI+x} ]; then My bash-foo is really not the best, but shouldn't there be a colon in there, i.e. ${GITLAB_CI:+x} ? Thomas > for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do > cp "$i" "$DEST_DIR/lib/" > done >
On 210907 1432, Thomas Huth wrote: > On 07/09/2021 13.08, Alexander Bulekov wrote: > > /src/build.sh: line 76: GITLAB_CI: unbound variable > > Fix that. > > > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > > --- > > > > This change is in preparation to revert: > > 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz. > > Reverting as-is produces an unbound variable complaint when we try to > > build the fuzzers in the OSS-Fuzz container. > > > > scripts/oss-fuzz/build.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh > > index 98b56e0521..5ddc769c9c 100755 > > --- a/scripts/oss-fuzz/build.sh > > +++ b/scripts/oss-fuzz/build.sh > > @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then > > "\nFor example: CC=clang CXX=clang++ $0" > > fi > > -if [ "$GITLAB_CI" != "true" ]; then > > +if [ -z ${GITLAB_CI+x} ]; then > > My bash-foo is really not the best, but shouldn't there be a colon in there, > i.e. ${GITLAB_CI:+x} ? I think the difference is that GITLAB_CI+x only checks if GITLAB_CI is set, while GITLAB_CI:+x checks that it is set and non-null. https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02 I don't think that makes much of a difference here. -Alex > > Thomas > > > > for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do > > cp "$i" "$DEST_DIR/lib/" > > done > > >
On 07/09/2021 14.51, Alexander Bulekov wrote: > On 210907 1432, Thomas Huth wrote: >> On 07/09/2021 13.08, Alexander Bulekov wrote: >>> /src/build.sh: line 76: GITLAB_CI: unbound variable >>> Fix that. >>> >>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu> >>> --- >>> >>> This change is in preparation to revert: >>> 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz. >>> Reverting as-is produces an unbound variable complaint when we try to >>> build the fuzzers in the OSS-Fuzz container. >>> >>> scripts/oss-fuzz/build.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh >>> index 98b56e0521..5ddc769c9c 100755 >>> --- a/scripts/oss-fuzz/build.sh >>> +++ b/scripts/oss-fuzz/build.sh >>> @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then >>> "\nFor example: CC=clang CXX=clang++ $0" >>> fi >>> -if [ "$GITLAB_CI" != "true" ]; then >>> +if [ -z ${GITLAB_CI+x} ]; then >> >> My bash-foo is really not the best, but shouldn't there be a colon in there, >> i.e. ${GITLAB_CI:+x} ? > > I think the difference is that GITLAB_CI+x only checks if GITLAB_CI is > set, while GITLAB_CI:+x checks that it is set and non-null. > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02 > > I don't think that makes much of a difference here. TIL, and I agree that it does not make a difference here (and if it would, your variant is certainly better). Reviewed-by: Thomas Huth <thuth@redhat.com>
On 07/09/21 13:08, Alexander Bulekov wrote: > > -if [ "$GITLAB_CI" != "true" ]; then > +if [ -z ${GITLAB_CI+x} ]; then I would slightly prefer to have "${GITLAB_CI+x}", since "test" in general doesn't like parameters that go away: $ [ = abc ] bash: [: =: unary operator expected What you wrote however works, so it's okay. Thanks, Paolo
On Wednesday, 2021-09-08 at 08:06:27 +02, Paolo Bonzini wrote: > On 07/09/21 13:08, Alexander Bulekov wrote: >> >> -if [ "$GITLAB_CI" != "true" ]; then >> +if [ -z ${GITLAB_CI+x} ]; then > > I would slightly prefer to have "${GITLAB_CI+x}", since "test" in > general doesn't like parameters that go away: > > $ [ = abc ] > bash: [: =: unary operator expected > > What you wrote however works, so it's okay. > If we are certain that the script is running a bash variant then we really should be using the '[[ ... ]]' variant of test which doesn't have that limitation since it can handle an empty or unset variable correctly. This doesn't appear to be the assumption though. Thanks, Darren.
diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh index 98b56e0521..5ddc769c9c 100755 --- a/scripts/oss-fuzz/build.sh +++ b/scripts/oss-fuzz/build.sh @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then "\nFor example: CC=clang CXX=clang++ $0" fi -if [ "$GITLAB_CI" != "true" ]; then +if [ -z ${GITLAB_CI+x} ]; then for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do cp "$i" "$DEST_DIR/lib/" done
/src/build.sh: line 76: GITLAB_CI: unbound variable Fix that. Signed-off-by: Alexander Bulekov <alxndr@bu.edu> --- This change is in preparation to revert: 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz. Reverting as-is produces an unbound variable complaint when we try to build the fuzzers in the OSS-Fuzz container. scripts/oss-fuzz/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)