Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip multi-release source sets in idea project import by default #107123

Merged

Conversation

mark-vieira
Copy link
Contributor

@mark-vieira mark-vieira commented Apr 4, 2024

There is an existing IntelliJ bug that prevents doing a full project build when source sets for multi-release jars are present. This changes the project import behavior so that these source sets are ignored by default and can be explicitly enabled by adding org.gradle.mrjar.idea.enabled=true to your ~/.gradle/gradle.properties file should you need to actively work on that code.

@mark-vieira mark-vieira added >non-issue :Delivery/Build Build or test infrastructure labels Apr 4, 2024
@mark-vieira mark-vieira requested a review from rjernst April 4, 2024 19:19
@mark-vieira mark-vieira requested a review from a team as a code owner April 4, 2024 19:19
@elasticsearchmachine elasticsearchmachine added Team:Delivery Meta label for Delivery team v8.14.0 labels Apr 4, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@@ -130,7 +130,8 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') {
':server:generateModulesList',
':server:generatePluginsList',
':generateProviderImpls',
':libs:elasticsearch-native:elasticsearch-native-libraries:extractLibs'].collect { elasticsearchProject.right()?.task(it) ?: it })
':libs:elasticsearch-native:elasticsearch-native-libraries:extractLibs',
':x-pack:libs:es-opensaml-security-api:shadowJar'].collect { elasticsearchProject.right()?.task(it) ?: it })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a random bug I happened to catch while testing this. This artifact is required to properly compile the project.

@@ -239,20 +240,22 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') {
* but before the XML document, e.g. a doctype or comment
*/
void modifyXml(Object path, Action<? super Node> action, String preface = null) {
Node xml = parseXml(path)
action.execute(xml)
if (project.file(path).exists()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this lenient since there seems to be some kind of race condition between when these tasks run and these configuration files exist. There's no good solution for this that I've found but at least doing a refresh or subsequent import will apply the missing config. This is really only applicable to fresh project imports on an empty workspace.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mark-vieira mark-vieira added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 4, 2024
@elasticsearchmachine elasticsearchmachine merged commit b3b4214 into elastic:main Apr 4, 2024
15 checks passed
@mark-vieira mark-vieira deleted the optional-mrjar-sourcesets branch April 4, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants