-
Notifications
You must be signed in to change notification settings - Fork 45
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
Improve RepositoryHelper.getSharedBundlePools to load all agent pools #384
Conversation
...x.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java
Outdated
Show resolved
Hide resolved
d9e11c9
to
eb2cae8
Compare
FYI, this change is necessary because Oomph as moved the location of the encoding.info file so at this point it's arguably a bug fix. That said, of course I do appreciate careful reviews! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the follow up.
Just one minor suggestion below (but probably also a matter of taste).
...x.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java
Outdated
Show resolved
Hide resolved
Oomph has moved the encoding.info to be in the same folder as the agents.info which lists all the agent locations (rather than in the folder of the default agent's location). The agents.info file can be loaded to determine the location of the pools.info of each agent. This refined approach covers the full generality of Oomph's shared agent manager allows all pools of all agents to be located with no assumptions about defaults. eclipse-equinox#373
eb2cae8
to
8bb17f5
Compare
I haven't analyzed yet, but it seems this PR might caused some severe problem, look at the test results: https://download.eclipse.org/eclipse/downloads/drops4/I20231111-0600/testResults.php Unfortunately I only found errors that errors happen on startup, but no real log fie, see
|
In this log it says it can't find pde.core?
|
I’m not sure how this can be in any way related. This method would only be used only after resolution succeeds but resolution itself is failing. And I’m very sure the method returns an empty list in an environment where oomph has not populated the user.home folder. |
I simply don't see other changes merged... @laeubi : any tycho / surefire updates? |
Note too in particular that only pde calls this method and only during target platform resolutions. |
@iloveeclipse platform uses a released version of Tycho and as far as I know the eclipse testframework uses ant/pdebuild anyways (@akurtakov ?) so I have no clue how this change or Tycho can affect this, maybe a concurrent build or network problem? |
Unless some PDE target platform resolution test is involved, it’s not even theoretically possible for the method to be called. |
Probably it’s a good idea to kick the build again. |
Let see if other builds will show same issue. If not, it was a glitch in the Matrix and we can restart Mac build. |
I see other builds were OK, so I will try to restart Mac build |
Next build was OK. So it was some infrastructure issue... |
Oomph has moved the encoding.info to be in the same folder as the agents.info which lists all the agent locations (rather than in the folder of the default agent's location). The agents.info file can be loaded to determine the location of the pools.info of each agent. This refined approach covers the full generality of Oomph's shared agent manager allows all pools of all agents to be located with no assumptions about defaults.
#373