Skip to content

Use a CachingOutputStream when using the build context #64

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 22, 2023

Conversation

gnodet
Copy link
Member

@gnodet gnodet commented May 22, 2023

No description provided.

@laeubi
Copy link
Collaborator

laeubi commented May 22, 2023

@gnodet can you explain what will be the advantage here?

@gnodet gnodet requested a review from kwin May 22, 2023 11:51
@gnodet
Copy link
Member Author

gnodet commented May 22, 2023

@gnodet can you explain what will be the advantage here?

It provides caching. The output file is not modified unless there's an actual change in the content.
My final goal is to have most plugins behave nicely when nothing has changed, so that jars are not modified, as it has a cascading effects in big reactor projects and cause a lot of rebuilds for nothing.

@laeubi
Copy link
Collaborator

laeubi commented May 22, 2023

Maybe you can adjust the commit message to explain this as it is not really obvious (for me) so one can read about it later on. I must confess the name is a bit confusing because one usually more have caching for reading data.

@gnodet
Copy link
Member Author

gnodet commented May 22, 2023

Maybe you can adjust the commit message to explain this as it is not really obvious (for me) so one can read about it later on. I must confess the name is a bit confusing because one usually more have caching for reading data.

Done

Copy link
Collaborator

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Looks good, but keep in mind that as of today no one is using the "new" build API here so a release might not have any impact on existing users.

@gnodet
Copy link
Member Author

gnodet commented May 22, 2023

Looks good, but keep in mind that as of today no one is using the "new" build API here so a release might not have any impact on existing users.

Yes, PRs will follow...

@laeubi
Copy link
Collaborator

laeubi commented May 22, 2023

Looks good, but keep in mind that as of today no one is using the "new" build API here so a release might not have any impact on existing users.

Yes, PRs will follow...

This won't help as this will instantly break the plugin, first m2e needs to add support for the new plexus-build API ...

@gnodet
Copy link
Member Author

gnodet commented May 22, 2023

Looks good, but keep in mind that as of today no one is using the "new" build API here so a release might not have any impact on existing users.

Yes, PRs will follow...

This won't help as this will instantly break the plugin, first m2e needs to add support for the new plexus-build API ...

Plugins won't be broken, I think it's only the incremental build inside m2e which will be broken, and most probably, not really broken, but running sub-optimally (i.e. will rebuild more than needed) until m2e is update to support both.

@laeubi
Copy link
Collaborator

laeubi commented May 22, 2023

Plugins won't be broken, I think it's only the incremental build inside m2e which will be broken, and most probably, not really broken, but running sub-optimally (i.e. will rebuild more than needed) until m2e is update to support both.

Well that's the purpose of the build-api if that is not working the plugin is (from user POV) broken...

The CachingOutputStream provides a write cache so that the target file is only modified if there's a content change.  If the data written exactly maps the existing content of the file, the file will not be modified at all.
@gnodet gnodet merged commit 7a142c9 into codehaus-plexus:master May 22, 2023
@gnodet gnodet deleted the cache-stream branch May 23, 2023 07:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants