Activated profiles should not always deactivate POM profiles that are active by default

[crankydillo]

These versions resolve with Maven. If I'm misuing the test APIs, I can create a sample project.

• Maven settings with profiles fails to active other profiles from parent activeByDefault #4269

[crankydillo]

Hi. We're back on this.

Has there been any work on profile deactivation: mvn -P-activated-profile or mvn -D '!activated-profile, where those profiles would not be tagged as active even if they would be activated in some other way (e.g. activeByDefault)? I have changed some code in rewrite-maven, and my tests, which do not explore this idea, pass and no existing one fails. However, when I add tests around disabling profiles those fail as it still views them as active. I can see the flaws in my code about this, but I haven't found any old code that handles this.

I searched a bit in PRs and issues. 'profile deactivation', 'profile disabling', and 'deactivation' didn't turn up anything.

[crankydillo]

@timtebeek I pinky promise not to sidetrack (far) on this, but got a quick solve for this (and in -java-17, -java-21)?

$ ./gradlew :rewrite-java-11:build
Starting a Gradle Daemon, 1 busy and 1 incompatible Daemons could not be reused, use --status for details

> Configure project :
Inferred project: rewrite, version: 8.29.0-SNAPSHOT

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':rewrite-java-11:compatibilityTest'.
> Could not resolve all dependencies for configuration ':rewrite-java-11:compatibilityTestRuntimeClasspath'.
   > Could not find org.junit.platform:junit-platform-launcher:.
     Required by:
         project :rewrite-java-11

My skills, internet, AI, and colleagues have all failed so far. We're primarily a Maven shop. Trying to regress before pushing the changes. I did check your build guidance.

[timtebeek]

Hi @crankydillo I've been traveling since Wednesday, getting home Sunday. I'll try to have a more detailed look next week. As for your IDE setup, one tip is to create a copy of this file to limit what modules are loaded. That could help speed up development as well.

rewrite/IDE.properties.tmp

Lines 1 to 7 in 537d023

	########################################################
	# If you are only working on a subset of the modules in this project, you can optimize your
	# IDE to only load those modules. Copy `IDE.properties.tmp` to `IDE.properties` and comment out
	# any lines that correspond to modules you do not want to work on. This will cause Gradle to
	# swap those project dependencies for binary dependencies resolved from either Maven local or
	# the OSS snapshots repository, and speed up your IDE.
	########################################################

I don't recall any work towards explicit profile deactivation. What kind of usage patterns do you see there beyond what Maven already does for us when you execute the rewrite-maven-plugin with those profiles enabled/disabled?

[crankydillo]

I don't recall any work towards explicit profile deactivation. What kind of usage patterns do you see there beyond what Maven already does for us when you execute the rewrite-maven-plugin with those profiles enabled/disabled?

Sorry, I missed this question. It's probably best for me to just add a test for this. I'll get that done soon. I don't think I pushed it as part of this PR (because it currently fails:).

Update: I have already pushed it. It's just @Disabled. You can find it here. Got to be honest, this isn't a big use case for us, at least not at this time. I just don't want to break existing, correct behavior in this case.

[github-actions]

Some suggestions could not be made:

• rewrite-gradle/src/main/groovy/RewriteSettings.groovy
  • lines 31-36

[github-actions]

Some suggestions could not be made:

• rewrite-gradle/src/main/groovy/RewriteSettings.groovy
  • lines 31-36

[timtebeek]

Thanks for the fresh work on this @crankydillo ; let me know when this is ready for review, as currently it's still marked as a draft.

[crankydillo]

I have it as draft, because there's still a bit of work (see here). I'll finish that up if you think the approach looks good.

Update: I'll just go ahead and do that. Should have it out of draft by tomorrow!

[crankydillo]

This test is related to my understanding of this Maven doc. It is related, but not the same as what I've written up in #4269, so thinking about creating a new issue for it. If you think I've misunderstood something or this test is invalid, we can just remove. This is not a current use-case for us.

[timtebeek]

Thanks for the rework @crankydillo ! Much appreciate the narrower scope as that makes this easier to review. Quick headsup though that I'll be mostly out this coming week ahead of JFall, but hope to revisit the week after.

[crankydillo]

:bump

[github-actions]

Some suggestions could not be made:

• rewrite-gradle/src/main/groovy/RewriteGradleProject.groovy
  • lines 81-81

[crankydillo]

Sorry to keep coming back to this, but I need to know if you will make a call on this soon. It has been in your 'ready-to-review' column for quite some time:)

We have lots of complicated POMs and our workaround for this (using a hack settings.xml with mirrors) is not working in many, many cases. I'm now working on hack 4 attempt to work around this. Honestly, I'm starting to think if forking and somehow keeping up to date is better than my hack attempts:). But having been bitten by that multiple times, I'm trying desperately to avoid it.

[timtebeek]

Understandable @crankydillo ! Maybe @MBoegers can fit in a review? I'm out this week, and it's been hard to get to everything.

[MBoegers]

On my list for tomorrow

[timtebeek]

Thanks a lot for your patience and the rework here @crankydillo ! Took a lot longer to review than I'd have hoped, but I'm happy with the solution you ended up with. I've added some light polish and with that we're good to merge I think, as soon as the tests pass.