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

Add Oomph's bundle pools to PDE's artifact repositories for resolution #878

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

merks
Copy link
Contributor

@merks merks commented Nov 4, 2023

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Very nice!
I thought about that already multiple times too and assumed that this couldn't be that hard but I'm surprised that it is that easy (you just have to know how 😄)

Copy link

github-actions bot commented Nov 4, 2023

Test Results

   264 files   -        6     264 suites   - 6   41m 31s ⏱️ - 11m 8s
3 328 tests +       1  3 297 ✔️ ±       0  30 💤 ±  0  1 +1 
7 557 runs   - 2 721  7 490 ✔️  - 2 698  66 💤  - 24  1 +1 

For more details on these failures, see this check.

Results for commit 5c54fd8. ± Comparison against base commit 84b2e1e.

♻️ This comment has been updated with latest results.

Copy link
Member

@HannesWell HannesWell 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. Thank you.

For clarification: with this change PDE only re-uses artifacts from the shared bundle-pool in the user.home if they are present, but does not add missing ones, doesn't it?

So I would say this is a good step towards #875, but not a full resolution?

@merks
Copy link
Contributor Author

merks commented Nov 5, 2023

I'm not sure what you mean by "missing ones". Oomph only has a single shared agent manager that is always located in ~/.p2. There is a fair bit more flexibility to the agent manager than that, but I expect pretty much no one in the world is using multiple agents. So I'm not sure what important thing you think might be missing.

@HannesWell
Copy link
Member

I'm not sure what you mean by "missing ones".

Artifacts not yet in the pool, that are included in a target and would need to be added to the pool.

@merks
Copy link
Contributor Author

merks commented Nov 5, 2023

Yes, it only allows p2 to read from the artifact/bundle pool, exactly like it does for all the other artifact repository URIs that are added by the existing artifact repository gathering logic.

PDE would need some major design changes and restructuring before it could be reasonably trusted (by me) to write into and reuse the shared bundle pool. For me, if that pool is corrupted, it would destroy more than 100 installations/workspaces.

@merks merks merged commit e7d05ff into eclipse-pde:master Nov 5, 2023
13 of 16 checks passed
@merks merks deleted the issue-875 branch November 5, 2023 12:36
@HannesWell
Copy link
Member

PDE would need some major design changes and restructuring before it could be reasonably trusted (by me) to write into and reuse the shared bundle pool. For me, if that pool is corrupted, it would destroy more than 100 installations/workspaces.

Yes, I just wanted to make it clear (for myself and others) what this change is exactly doing. :)

@merks
Copy link
Contributor Author

merks commented Nov 5, 2023

PDE would need some major design changes and restructuring before it could be reasonably trusted (by me) to write into and reuse the shared bundle pool. For me, if that pool is corrupted, it would destroy more than 100 installations/workspaces.

Yes, I just wanted to make it clear (for myself and others) what this change is exactly doing. :)

It does only exactly what the title says. 😁

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