The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #18077 (net: Add NAT-PMP port forwarding support by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

What are the actual benefits of exporting DOCKER_NAME variable? Do we use it somehow?

Concept ACK.

What are the actual benefits of exporting DOCKER_NAME variable? Do we use it somehow?

It is used as argument for Docker (--name $DOCKER_NAME) to give the containers a concrete name instead of generating a random one, see #16623 (review).

It is used as argument for Docker (--name $DOCKER_NAME) to give the containers a concrete name instead of generating a random one, see #16623 (review).

I know it ;)
What are container concrete names used for? What is wrong with random ones? (All questions are in context of CI)

What are container concrete names used for? What is wrong with random ones? (All questions are in context of CI)

From my perspective it would be a convenience feature right now: whenever as developer you make non-trivial changes in the CI scripts you very likely need to reproduce the travis build locally (as described in e.g. https://github.com/erdc/proteus/wiki/Replicating-the-TravisCI-Environment-on-your-Local-Machine) to dig deeper and inspect the container in some way (show logs, kill it, restart it, execute a command inside it etc. etc....) . For that it is way more comfortable to have deterministic names instead of the need to always find out the container hash or random name via docker ps first.

From my perspective it would be a convenience feature right now: whenever as developer you make non-trivial changes in the CI scripts you very likely need to reproduce the travis build locally (as described in e.g. https://github.com/erdc/proteus/wiki/Replicating-the-TravisCI-Environment-on-your-Local-Machine) to dig deeper and inspect the container in some way (show logs, kill it, restart it, execute a command inside it etc. etc....) . For that it is way more comfortable to have deterministic names instead of the need to always find out the container or random name via docker ps first.

In case a developer keeps travis docker containers locally, it is up to her to patch scripts locally.

IMHO, no need for these changes to the repository (if the only use case is local running).

Just a hint that how to run locally is described here: https://github.com/bitcoin/bitcoin/tree/master/ci#running-a-stage-locally

Travis only provides the compute to use, we don't rely on any of its software

In case a developer keeps travis docker containers locally, it is up to her to patch scripts locally.

IMHO, no need for these changes to the repository (if the only use case is local running).

While I agree that the changes are not strictly needed right now, I'd always prefer a reproducible behaviour to random behaviour in CI/testing, even if it concerns only something trivial as just container names.

Just a hint that how to run locally is described here: https://github.com/bitcoin/bitcoin/tree/master/ci#running-a-stage-locally

I'm aware of that, but just because all CI stages pass on a local (highly individual) development environment doesn't necessarily mean it also does on the Travis environment. While working on #17900 locally everything was passing fine, but on Travis it failed. I didn't see any alternative as to recreate the Travis environment in a Docker container locally to reproduce exactly what's going an and what is different to my local development machine (that's why I wrote "non-trivial" changes in my reasoning).

Travis only provides the compute to use, we don't rely on any of its software

Quick counter-example: from where would the testing scripts get the variable $TRAVIS_OS_NAME from without Travis (set in .travis.yml)? I highly doubt that the CI setup as it is right now is as decoupled from Travis than most people think (primarily because it is almost never run locally by anyone, only from people making changes).

Quick counter-example: from where would the testing scripts get the variable $TRAVIS_OS_NAME from without Travis (set in .travis.yml)? I highly doubt that the CI setup as it is right now is as decoupled from Travis than most people think (primarily because it is almost never run locally by anyone, only from people making changes).

I do test all linux builds locally on an arm machine continuously. TRAVIS_OS_NAME is only used to distinguish osx from linux, and should work fine unless you want to run the osx build locally.

I don't really understand the push back here. This is straightforward, and makes it easier to identify containers when you're running tests locally. I can't see why that's something you should have to "patch scripts locally." to do. However if this is so controversial it can just be closed.

@fanquake

However if this is so controversial it can just be closed.

Please don't close.

This is straightforward, and makes it easier to identify containers when you're running tests locally.

It was not clear for me from the OP. Now I'm convinced that these changes are useful.

Concept ACK.
Could the PR description be more descriptive with adding "makes it easier to identify containers when you're running tests locally"?

Could the PR description be more descriptive

I've updated the PR description.

though I'd prefer to use the ci_ prefix for the name

Done, and addressed other comments.

ACK 9e111db