Cleanup (ShellCheck, whitespace unification)

[Simran-B]

1. Not sure what your stand is on whitespace changes. I'm happy to revert those.

2. But using double quotes around paths seems like a good idea - if e.g. $DOWNLOAD_DIR was changed to a path with a space in it, it would break.

3. Using quotes here is a bit too much, so I did no change it: make -j$(nproc)

4. In cases like sh bootstrap.sh ${BOOTSTRAP_ARGS}, there are intentionally no quotes I suppose.

5. Not sure about the following in scripts\base\build_boost.sh:

   BOOST_MAJOR_MINOR=$(echo "${BOOST_VERSION}" | cut -d. -f-2)
   

   Possible misspelling: BOOST_VERSION may not be assigned, but BOOST_VERSION_U is (SC2153)

6. In scripts\common\install_sonar.sh there are two warnings:

   Useless echo? Instead of 'echo $(cmd)', just use 'cmd' (SC2005)

   Does the pattern serve any particular purpose? echo $(sonar-scanner --help)

7. I substituted `cmd` by $(cmd) as suggested

8. CI_COMMON_VERSION was set twice in ci-common/Dockerfile and I combined the two ENVs

[Simran-B]

Not sure how to interpret the build error:

#17 ERROR: executor failed running [/bin/sh -c export DOWNLOADS_DIR=/tmp/downloads &&     mkdir /tmp/downloads &&     source /tmp/versions_common.sh &&     /tmp/install_sonar.sh &&     /tmp/install_ccache.sh &&     rm -rf /tmp/downloads]: runc did not terminate sucessfully
------
 > [ci-common 9/11] RUN export DOWNLOADS_DIR=/tmp/downloads &&     mkdir /tmp/downloads &&     source /tmp/versions_common.sh &&     /tmp/install_sonar.sh &&     /tmp/install_ccache.sh &&     rm -rf /tmp/downloads:
------
failed to solve: rpc error: code = Unknown desc = failed to solve with frontend dockerfile.v0: failed to build LLB: executor failed running [/bin/sh -c export DOWNLOADS_DIR=/tmp/downloads &&     mkdir /tmp/downloads &&     source /tmp/versions_common.sh &&     /tmp/install_sonar.sh &&     /tmp/install_ccache.sh &&     rm -rf /tmp/downloads]: runc did not terminate sucessfully
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.9/x64/bin/aswfdocker", line 11, in <module>
    load_entry_point('aswfdocker==0.2.0', 'console_scripts', 'aswfdocker')()
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/decorators.py", line 73, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/click-7.1.2-py3.7.egg/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/aswfdocker-0.2.0-py3.7.egg/aswfdocker/cli/aswfdocker.py", line 170, in build
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/aswfdocker-0.2.0-py3.7.egg/aswfdocker/builder.py", line 88, in build
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'docker buildx bake -f /tmp/docker-bake-IMAGE-common-2.json --progress auto' returned non-zero exit status 1.

https://github.com/AcademySoftwareFoundation/aswf-docker/pull/74/checks?check_run_id=1146534800

[aloysbaillet]

Thanks @Simran-B ! That's a lot of double quotes ;-)
Happy to merge as it seems cleaner indeed, except for the export PATH change which seems it would have an effect on cmake subprocess, unless I'm mistaken?

[Simran-B]

Addressed your comment. The only open point is:

5. Not sure about the following in scripts\base\build_boost.sh:

   BOOST_MAJOR_MINOR=$(echo "${BOOST_VERSION}" | cut -d. -f-2)
   

   Possible misspelling: BOOST_VERSION may not be assigned, but BOOST_VERSION_U is (SC2153)

[aloysbaillet]

BOOST_VERSION version is coming from the scripts/XXXX/versions_base.sh set of files and is indeed not defined in this script, but that's expected.

[aloysbaillet]

Looks good, thank!

[aloysbaillet]

Looks like there's an error on build here:

#17 1.680 + unzip /tmp/downloads/sonar-bw.zip
#17 1.685 Archive:  /tmp/downloads/sonar-bw.zip
#17 1.685    creating: build-wrapper-linux-x86/
#17 1.685   inflating: build-wrapper-linux-x86/libinterceptor-x86_64.so  
#17 1.686   inflating: build-wrapper-linux-x86/libinterceptor-i686.so  
#17 1.686   inflating: build-wrapper-linux-x86/build-wrapper-linux-x86-64  
#17 1.704   inflating: build-wrapper-linux-x86/libinterceptor-haswell.so  
#17 1.705 + mv build-wrapper-linux-x86 /var/opt/.
#17 1.709 + ln -s /var/opt/build-wrapper-linux-x86/build-wrapper-linux-x86-64 /usr/bin/build-wrapper-linux-x86-64
#17 1.712 + build-wrapper-linux-x86-64 --help
#17 1.714 build-wrapper, version 6.12 (linux-x86)
#17 1.714 Copyright (C) 2014-2020 SonarSource SA, info@sonarsource.com
#17 1.714 
#17 1.714 Usage: build-wrapper-linux-x86-64 --out-dir <output directory> <build command>
#17 ERROR: executor failed running [/bin/sh -c export DOWNLOADS_DIR=/tmp/downloads &&     mkdir /tmp/downloads &&     source /tmp/versions_common.sh &&     /tmp/install_sonar.sh &&     /tmp/install_ccache.sh &&     rm -rf /tmp/downloads]: runc did not terminate sucessfully

[jfpanisset]

The problem happens when scripts/common/install_sonar.sh is called. Once the local SonarCloud scanner wrapper has been installed, it is called with --help to print out command line options:

echo $(build-wrapper-linux-x86-64 --help)

but it looks like build-wrapper --help returns a non-zero return code, and previously in the script:

set -ex

ensures that the script will abort on any errors. I imagine the same error would also happen a bit further down with:

echo $(sonar-scanner --help)

Before this PR, these commands were simply:

build-wrapper-linux-x86-64 --help
...
sonar-scanner --help

I'm not sure why running them in a subshell via $() would change the return value, but in this case the simplest way to fix this would seem to be to just revert to the original version. I have to admit I'm a bit puzzled as to why ShellCheck wants to wrap the call to a binary that prints something useful inside $(), that seems counter intuitive?

Looking at ShellCheck Style it seems to explicitly call out:

echo "$(date)"                    # Useless use of echo

[jfpanisset]

Inside an ASWF container if I try:

[root@2df788287386 tmp]# /usr/bin/build-wrapper-linux-x86-64
build-wrapper, version 6.10 (linux-x86)
Copyright (C) 2014-2020 SonarSource SA, info@sonarsource.com

Usage: /usr/bin/build-wrapper-linux-x86-64 --out-dir <output directory> <build command>
[root@2df788287386 tmp]# echo $?
10

it does appear that build-wrapper-linux-x86-64 returns a non-zero value when called with --help, but:

[root@2df788287386 tmp]# cat foo.sh
#!/bin/bash

set -e

echo $(build-wrapper-linux-x86-64  --help)
echo $?

echo "===================="

build-wrapper-linux-x86-64  --help
echo $?
[root@2df788287386 tmp]# ./foo.sh
build-wrapper, version 6.10 (linux-x86) Copyright (C) 2014-2020 SonarSource SA, info@sonarsource.com Usage: build-wrapper-linux-x86-64 --out-dir <output directory> <build command>
0
====================
build-wrapper, version 6.10 (linux-x86)
Copyright (C) 2014-2020 SonarSource SA, info@sonarsource.com

Usage: build-wrapper-linux-x86-64 --out-dir <output directory> <build command>
[root@2df788287386 tmp]# echo $?
10

it seems that the new code which wraps the called to build-wrapper-linux-x86-64 inside $() does not propagate the error to the calling shell, and the script does not abort, whereas when called directly, the script aborts and never prints the return value (I have to do that manually after the script aborts).

So if anything the changes should make the script work, not make it fail?

[Simran-B]

It actually makes sense. The exit code of the sub-command is only accessible from within $().

echo $(./build-wrapper-linux-x86/build-wrapper-linux-x86-64 --help; echo $?)
... 10
echo $(./build-wrapper-linux-x86/build-wrapper-linux-x86-64 --help); echo $?
...
0

Using command substitution suppresses the non-zero exit code. It's feels strange that it returns 10 in case of --help, but it actually prints the same text as when you don't pass any arguments, which is considered an error.

Reverted my change and added a note. sonar-scanner returns with 0, so not affected.

[jfpanisset]

I'm probably reading the previous build failure log incorrectly: https://github.com/AcademySoftwareFoundation/aswf-docker/runs/1162762399?check_suite_focus=true
It seems it is now building correctly.
I guess what I don't understand is how this build was working in the first place without having wrapped the call to build-wrapper-linux-x86-64.
A small nitpick is that looking at the binary itself, it does not actually take a --help command line parameter, so if what we are interested in doing is getting the usage message to come up in the logs, perhaps the code should just be:

echo $(build-wrapper-linux-x86-64)

[Simran-B]

what I don't understand is how this build was working in the first place without having wrapped the call to build-wrapper-linux-x86-64

It did have the command wrapped:

aswf-docker/scripts/common/install_sonar.sh

Line 16 in 8b1f49f

echo $(build-wrapper-linux-x86-64 --help)

I removed the wrapping echo $() as suggested by ShellCheck, but here it's actually necessary.

it does not actually take a --help command line parameter

Oh, the exit code makes a lot more sense then!

[aloysbaillet]

Thanks for the updates!

[aloysbaillet]

Would you be OK resolving the conflicts @Simran-B ?

[Simran-B]

@aloysbaillet Conflicts should be resolved now.

[aloysbaillet]

Thanks!