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

[resolve] Add cache mode #5304

Closed
wants to merge 2 commits into from
Closed

[resolve] Add cache mode #5304

wants to merge 2 commits into from

Conversation

pkriens
Copy link
Member

@pkriens pkriens commented Jun 30, 2022

cache – Will use a cache file in the workspace cache. If that file is stale relative to the workspace or project or it does not exist, then the bnd(run) file will be resolved and the result is stored in the cache file.

Signed-off-by: Peter Kriens Peter.Kriens@aqute.biz

cache – Will use a cache file in the workspace cache. If that file is stale relative to the workspace or project or it does not exist, then the bnd(run) file will be resolved and the result is stored in the cache file.




Signed-off-by: Peter Kriens <Peter.Kriens@aqute.biz>
Copy link
Member

@bjhargrave bjhargrave left a comment

Choose a reason for hiding this comment

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

Some comments/questions.

@@ -185,4 +194,100 @@ public Collection<Container> getRunbundles() throws Exception {
return super.getRunbundles();
}

private Collection<Container> cache() throws Exception {
File ours = getPropertiesFile();
Copy link
Member

Choose a reason for hiding this comment

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

What about included files in the bndrun file and in the workspace?

In the gradle plugin all of the files factor in to out-of-date checks:

private ConfigurableFileCollection bndConfiguration() {
Workspace bndWorkspace = bndProject.getWorkspace();
return objects.fileCollection()
.from(bndWorkspace.getPropertiesFile(), bndWorkspace.getIncluded(), bndProject.getPropertiesFile(),
bndProject.getIncluded());
}

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense

biz.aQute.resolve/src/biz/aQute/resolve/Bndrun.java Outdated Show resolved Hide resolved
biz.aQute.resolve/src/biz/aQute/resolve/Bndrun.java Outdated Show resolved Hide resolved
- Uses the project/workspace lastmodifiedtime, which includes include files
- Removed removal of container errors in test, uses a resolve file that has proper dependencies
- Added a testReason so the caching choice could be tested from outside
- Increased test coverage

Signed-off-by: Peter Kriens <Peter.Kriens@aqute.biz>
@pkriens pkriens mentioned this pull request Jul 1, 2022
@bjhargrave
Copy link
Member

I guess we can close this PR since you merged #5305.

@bjhargrave bjhargrave closed this Jul 1, 2022
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.

None yet

2 participants