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

[MNG-8005] do not ignore ide WorkspaceReader #1372

Closed
wants to merge 1 commit into from
Closed

[MNG-8005] do not ignore ide WorkspaceReader #1372

wants to merge 1 commit into from

Conversation

jonasrutishauser
Copy link
Contributor

This change should fix the regression introduced by https://github.com/apache/maven/pull/963/files#diff-c14736c8f0b57d9f26af7f81fff3114a217edbed2b42049aa3c746ddab38966eR245-R250

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@cstamas cstamas requested a review from gnodet January 10, 2024 20:14
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

@jonasrutishauser I don't really see what's broken with the mentioned commit...
Support for additional workspace readers has already been added through gnodet@a472975

The IDE workspace reader is already injected in the repository system session:

@Nullable @Named("ide") WorkspaceReader workspaceRepository,

@gnodet
Copy link
Contributor

gnodet commented Jan 11, 2024

@jonasrutishauser an IT would be welcomed

@jonasrutishauser
Copy link
Contributor Author

As requested I added a test: apache/maven-integration-testing#332

@jonasrutishauser
Copy link
Contributor Author

The IDE workspace reader is already injected in the repository system session:

@Nullable @Named("ide") WorkspaceReader workspaceRepository,

That's correct, but this injected reader is directly overwritten:

private CloseableSession newCloseableSession(MavenExecutionRequest request, WorkspaceReader workspaceReader) {
return repositorySessionFactory
.newRepositorySessionBuilder(request)
.setWorkspaceReader(workspaceReader)
.build();
}

@cstamas
Copy link
Member

cstamas commented Jan 18, 2024

Proposed PR that is a bit broader (but performs bigger and IMHO "proper" cleanup in this area): #1385

@gnodet gnodet closed this Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants