Stops CMake build output folders being named "default"

[betamaxbandit]

The default IBuildConfiguration is no longer used by projects that use ICBuildConfigurationProvider.

For CMake, Makefile and other projects the build output folder is sometimes named "default" rather than the pattern
toolName.launchMode.toolchain OS.toolchain Arch.launchTarget Id (eg: cmake.debug.win32.x86_64.Local). PR #1076 exposes new API (ICBuildConfigurationProvider.getCBuildConfigName) to encourage this naming pattern.

The "sometimes" is variable and often happens when a project is first created when the active launch target is Local and the launch mode is "run", but not always. This gives a random, inconsistent impression to CDT.

The Platform project always contains a IBuildConfiguration with the name IBuildConfiguration.DEFAULT_CONFIG_NAME. It seems the original Core Build system design went to some length to fit in with this and always make use of this IBuildConfiguration when pairing it with a new ICBuildConfiguration.

With this PR, this no longer happens, allowing CDT code to be simplified and the build folder naming made consistent, always adhering to ICBuildConfigurationProvider.getCBuildConfigName.

Addresses Issue: CDT CMake Improvements #1000, IDE-82683-REQ-024 Default CMake build folder

[betamaxbandit]

@jonahgraham ,
I have pushed a commit with failing junit tests for this.
The test is still WIP and it does pull in a number of dependencies into a single test. Let me know if that is a problem.

The fixes are not yet in place, but I have marked where I think changes should happen.
I'll do further testing on this before pushing the fixed PR.

There are a number of places in CDT code where the following line appears:

names.add(IBuildConfiguration.DEFAULT_CONFIG_NAME);

I need to investigate whether these can be removed too.

[github-actions]

Test Results

   636 files  +   35     636 suites  +35   36m 55s ⏱️ + 22m 56s
11 439 tests +1 225  11 295 ✅ +1 104  144 💤 +121  0 ❌ ±0 
11 454 runs  +1 202  11 312 ✅ +1 083  142 💤 +119  0 ❌ ±0 

Results for commit 5539a6e. ± Comparison against base commit e819f8a.

♻️ This comment has been updated with latest results.

[jonahgraham]

This pull request removes 1217

Please disregard this part of the Test Results method in #1084 (comment) - the removed tests are simply not run on this PR to make build run faster. #1002 is umbrella for adding better auto-comments to PRs.

[jonahgraham]

In case you have the cdt-lsp projects closed, make sure to take into account how it uses ICBuildConfigurationManager.getBuildConfiguration. For example here

[betamaxbandit]

Hi @jonahgraham ,
I've updated my draft PR with that actual implementation for this ticket.
I still have some tidy up to do in the JUnit tests.

[betamaxbandit]

@jonahgraham ,
my most recent PR fixes the problem I was having in the JUnits where the initial conditions for each test were not being reset. I've solved this now by adding a resetCoreBuildLaunchBarTracker(). This allows my tests to behave now as if each one were running in a new workspace and the resulting active build configuration is set as expected.

I'm very pleased with how this work has gone because I believe quarantining the default build config to the noConfigs bin is totally the correct thing to do. It stops the "default" name appearing in the build output directory sometimes. More importantly I think the correct build config for the active launchbar controls is being set straight away now, without relying on a build action first.

I have a little bit of tidying to do to the junits, but I think functionally they are complete.

[jonahgraham]

The failing test is known flaky - #1065 - I have restarted the build anyway.

[jonahgraham]

This LGTM - set to ready to review when you are satisfied.

[betamaxbandit]

@jonahgraham ,
I've updated the PR with your most recent suggestions.
And I've completed my todo items in the tests.
I think this is ready for review now.

[jonahgraham]

This looks good to me - it exposes a pre-existing bug in cdt-lsp I have raised here eclipse-cdt/cdt-lsp#435