Add apt wrapper scripts

[Flamefire]

Add scripts to securely add APT keys and repositories as wrappers over the raw commands. This fixes various issues with the approach in the current CI script. E.g. in

for key in "${SOURCE_KEYS[@]}"; do
    for i in {1..$NET_RETRY_COUNT}; do
        keyfilename=$(basename -s .key $key)
        curl -sSL --retry ${NET_RETRY_COUNT:-5} "$key" | sudo gpg --dearmor > /etc/apt/trusted.gpg.d/${keyfilename} && break || sleep 10
    done
done

• After all retries failed the code will continue instead of failing (same for adding repos)
• The write may fail with a permission denied if the executing user isn't already root: sudo applies to the command, not the redirection and hence doesn't help here as gpg --dearmor runs fine without it. One would need to pipe it into sudo tee or use gpg -o parameter (which I chose)
• Adding a key from a URL which doesn't contain a filename fails as basename can't extract something meaningful

[sdarwin]

The first thought which comes to mind, is that this is a bash script calling a bash script. It could be inline, instead of creating external wrapper scripts.

When developing Drone, the feedback I got was to reduce the number of scripts to the absolute maximum. The reason there is more than one is due to multiple languages: Python/Starlark calling Bash or powershell.

Perhaps each option has pros and cons.

---

We have seen multiple situations where apt-add-repository fails. The most recent in Slack when pdimov pointed out that Drone failed on ubuntu 14.04. That can be solved by removing apt-add-repository completely. Yet this pull request is still using apt-add-repository.

Perhaps the new functions here are more generalized. They can install any SOURCES or SOURCE_KEYS.

We specifically need to install "ppa:ubuntu-toolchain-r/test". With that ppa in mind, it's possible to completely remove apt-add-repository and ensure a more robust result.

On the other hand, any bugs reported about ubuntu:14.04 shouldn't be taken too seriously because the operating system is close to end-of-life.

[Flamefire]

The first thought which comes to mind, is that this is a bash script calling a bash script. It could be inline, instead of creating external wrapper scripts.

This allows reuse across CIs. Otherwise we'd need to copy those functions and the calling code to each CIs config file obfuscating the actual testing logic by adding those ~30 lines of code
Furthermore this allows centralized maintenance as e.g. the currently in-use approach has a bug. If it was a bash script already we could simply fix it in once place.

I see that as big pros for this option.

We have seen multiple situations where apt-add-repository fails. The most recent in Slack when pdimov pointed out that Drone failed on ubuntu 14.04. That can be solved by removing apt-add-repository completely. Yet this pull request is still using apt-add-repository.

I see in https://github.com/boostorg/boost-ci/blob/master/ci/drone/linux-cxx-install.sh that there is a special add_repository_toolchain used in addition to add_repository (the latter also calls apt-add-repository)
So I'd say this usually works and we can keep using it.

Perhaps the new functions here are more generalized. They can install any SOURCES or SOURCE_KEYS.

Yes. Again the centralized approach also allows centralized feature addition for e.g. adding keys from keyserver-URLs instead of only path-like URLs as before which would otherwise need updates to all existing configs.

We specifically need to install "ppa:ubuntu-toolchain-r/test". With that ppa in mind, it's possible to completely remove apt-add-repository and ensure a more robust result.

Sorry I don't understand that: We (may) not only add that single PPA but others too and for that we do need/are able to use apt-add-repository, don't we?
And for the current issue with that specific PPA: We could update the script to do something special for that single PPA, e.g. automatically invoking some form of add_repository_toolchain in that script.

BTW: I've seen even Ubuntu 18.04 failing strangly with a similar error.

[sdarwin]

failing strangely

If you had drone jobs configured on boostorg/locale then we could compare if linux-cxx-install.sh was more successful than the regular github actions using apt-add-repository during the same timeframe.

update the script to do something special for that single PPA

Yes, it's possible. Check to see that single PPA, and take customized steps only for that one.

[Flamefire]

If you had drone jobs configured on boostorg/locale then we could compare if linux-cxx-install.sh was more successful than the regular github actions using apt-add-repository during the same timeframe.

I started trying to add drone jobs some time ago but it was a bit more difficult than I could handle in the available time back then as locale needed a bit of other stuff too for CI. We have this repo here for comparison but CI here doesn't run often due to few changes...

The changes here are an improvement as they fix actual issues aren't they?
So how do we want to go forward from here?

[sdarwin]

to go forward from here?

Option 1:

The following seems like the most robust solution. In do_add_repository evaluate a regex match on $name.

if it matches "ubuntu-toolchain-r" include the logic from add_repository_toolchain , which is ultimately two lines of code: Add content to /etc/apt/sources.list.d .

else run apt-add-repository

Option 2:

Merge the current PR without the above mentioned logic, is also ok.

[sdarwin]

Update: I see it's actually more than two lines of code, it must compute VERSION_CODENAME. My suggestion is to place that logic in do_add_repository. But if you don't feel confident about this methodology, it is not required right now.

[Flamefire]

I would merge that now and improve the script in a separate PR if all are fine with the script-approach.

What might need discussing: In the GHA CI file I added

            SOURCE_KEYS+=('http://keyserver.ubuntu.com/pks/lookup?op=get&search=0x1E9377A2BA9EF27F')
            SOURCES+=(ppa:ubuntu-toolchain-r/test)

to explicitly download the key to avoid apt-add-repository failing. With add_repository_toolchain we might not need that in the GHA CI file but have to do it implicitly in the script. Which would be an argument for doing the change in this PR.
I guess that needs a general decision: Should we download the key(s) ourselves from inside do_add_repository for the "bypass-code" or let users do that?

In do_add_repository evaluate a regex match on $name.

Why a regex and not a direct match on "ppa:ubuntu-toolchain-r/test"? Or did you wanted to also allow e.g. "ppa:ubuntu-toolchain-r/ppa"?

[sdarwin]

The functionality of add-apt-keys.sh and add-apt-repositories.sh is closely enough related that it could all be merged in one wrapper file, instead of having two wrapper files. If that occurred, logic surrounding ubuntu-toolchain-r could be handled more simply instead of breaking it into multiple parts. When the toolchain is detected, add both keys and repos, at the same moment. Otherwise proceed to the other steps. At least an idea.

Why a regex and not a direct match

I did not investigate if the ppa is 100% consistently named and thus a direct match is better than a regex. Maybe I was thinking that a "partial match" on "ubuntu-toolchain-r" would catch everything. Whichever you find to be better.

[Flamefire]

The functionality of add-apt-keys.sh and add-apt-repositories.sh is closely enough related that it could all be merged in one wrapper file, instead of having two wrapper files.

But adding keys and repositories are different ops and the current CI files do use them separately (SOURCE_KEYS & SOURCES)

If that occurred, logic surrounding ubuntu-toolchain-r could be handled more simply instead of breaking it into multiple parts.

For that specific case one file can call the other. But if we merged them, how would we handle (existing) uses cases of e.g. adding LLVM repos? See e.g. https://github.com/boostorg/test/blob/a6063692a714d87584e9b9a940a8633b8cb6479a/.github/workflows/ci.yml#L245-L248

My idea was to provide those wrapper scripts as simple replacements of the underlying command with minor additions commonly used, e.g. retry & adding multiple "elements", such that Boost authors have good flexibility.

[sdarwin]

Perhaps merging them in one wrapper script doesn't match the idea you have in mind, and isn't the ideal solution. That is fine. If it were done, the "how" is to pass in multiple arguments such as SOURCE_KEYS & SOURCES via multiple command-line arguments to the function, or global variables or environment variables.

My idea was to provide those wrapper scripts as simple replacements of the underlying command

alright.