Add missing services to CI detection and make it consistent between RubyGems and Bundler

[nevinera]

What was the end-user or developer problem that led to this PR?

There were two separate lists of environment variables to maintain that represented our ability to determine the CI environment, which made maintaining and updating them difficult.

What is your fix for the problem, implemented in this PR?

I've introduced the Gem::CIDetector, a single abstraction responsible for "detecting the CI environment (if any)". It exposes ci? and ci_strings methods, which represent the usages the two existing sites have, and it clearly documents the provenance and expectations it makes about the presence of specific environment variables for various CI providers.

It also introduces a few effective behavioral changes (though I could slice those off into another PR if preferred):

• We'll consider ourselves to be on a CI in more cases - if CI_NAME or TASKCLUSTER_ROOT_URL are set specifically, since those represent two cases that were either overlooked or are no longer covered by the existing implementation. (Or possibly TaskCluster still does provide RUN_ID, but failed to document it)
• We will notice/track a few additional services in ci_strings (cirrus, dsari, taskcluster), stop tracking 'snap' (they went under in 2017), and update buildbox to buildkite (they've been called that for 8 years, and the BUILDBOX envs have been deprecated for 3).
• We'll also sort/uniq/downcase the values (all of which only matter because of the special case of CI_NAME - probably only the uniq will matter in practice, as some of the providers supply CI_NAME alongside their own named ENV.

I also found this more complete list, and would be happy to build CIDetector out a bit more (here or in another PR) if you'd like.

(Resolves #6038)

[martinemde]

This looks like a clear improvement, and the maintenance cost of duplication is no worse than before (with an eventual offramp to a single source). I'm for merging this.

[martinemde]

Heheh, test failures on CI. Who could have predicted 😆

[nevinera]

Heheh, test failures on CI. Who could have predicted 😆

.. Right. Because ENV["CI"] IS set here, and so are the github-actions ones.
The test-helper I wrote in the other PR overrode the entire env, but the one we're using here doesn't do that. I'll update these specs to handle ENV a little more thoroughly.

[deivid-rodriguez]

Just making sure my previous suggestion was understood. The idea was that bundler/lib/bundler/ci_detector.rb would not just duplicate lib/rubygems/ci_detector.rb but that it would try to load the RubyGems module if available and duplicate it if not (see the rescue NamError dance in my suggested code).

Also, I was not suggesting any automated commit. Just that a rake tasks that updates bundler/lib/bundler/ci_detector.rb from the contents of lib/rubygems/ci_detector.rb could be run in CI to make sure it doesn't generate any source control changes (in other words, that the files are in sync).

That all said, I think by having (almost) exact duplicates like your did here, it's even easier to make sure they stay in sync, so this seems perfectly fine too.

I would just maybe add a TODO note about when we will be able to completely get rid of the duplicated logic (i.e., once RubyGems 3.4 support is dropped from Bundler).

[nevinera]

Just making sure my previous suggestion was understood. The idea was that bundler/lib/bundler/ci_detector.rb would not just duplicate lib/rubygems/ci_detector.rb but that it would try to load the RubyGems module if available and duplicate it if not (see the rescue NamError dance in my suggested code).

@deivid-rodriguez - Ah, I missed the NameError bit at the top actually, scanned right past it in the browser. Now that I've understood though, I think I'd prefer not to do such a check-and-switch though; if we enforce/keep the files in sync there shouldn't be any benefit, and it does make the check a bit more complex.

I do understand your rake approach (though I misread "make sure they are kept in sync in CI" to involve committing), and I'm willing to implement that instead if you prefer it. I started to initially, but there was enough code involved that it didn't feel like it would be worthwhile for this target (based on the history it'll get 1-2 changes per year, and manually duplicating them won't be much work). But if we decide to use this approach more frequently (in maybe two more locations?), I think that's the approach we should take for generalizing it, possibly even with a directory of templates to generate both implementations from.

(Edit: Unless there's already some framework in place to leverage for that templating?)

[deivid-rodriguez]

Nah, I just wanted to explain what my suggestion was, but I think the approach you went with works just fine. I'd just make sure to add a note to remind us when the duplicates can be removed.

[nevinera]

Nah, I just wanted to explain what my suggestion was, but I think the approach you went with works just fine. I'd just make sure to add a note to remind us when the duplicates can be removed.

Is there any kind of process for finding all the stuff that can be cleaned up after each version-drop? Or is it just an incremental "oh, we don't support 3.4 anymore, so I guess I can clean this up"?

[deivid-rodriguez]

No, no formal process. I usually make sure to include notes with the oldest version supporting something, so that I can grep for it when dropping support for old versions.

[nevinera]

Oh that's the sort of process I meant :-)

[martinemde]

One last little change. I think this should merge before the end of year release, if possible. Agree, @deivid-rodriguez?

[deivid-rodriguez]

One last little change. I think this should merge before the end of year release, if possible. Agree, @deivid-rodriguez?

Yes, this is good to ship!

[martinemde]

Thank you for this improvement, @nevinera.

[simi]

Sorry for being late to this party. It looks amazing. 💪