-
Notifications
You must be signed in to change notification settings - Fork 41
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
Use RepositoryTracker for KnownRepositories to be consistent with Update #513
Use RepositoryTracker for KnownRepositories to be consistent with Update #513
Conversation
This is another behavior change where we don't really know who or what relies on the old behavior. More concerning is that it's just not immediately clear where are all the places that the system repositories will still surface. Certainly this constant is not much used: But this one is widely used, with the selected one being the one I mentioned before and the one that you are addressing in this PR: So that makes me question, where else does this crop up? While exploring with the debugger, I see that when installing something new, the planner sees many more repositories than meet the eye: All of this begs the question, what exactly is a system repository? Who is supposed to add such a thing, the system? Who is supposed to see them, the underlying frameworks but not the user? What if downstream application developers add system repositories for some specific purpose for impacting update and install and now we make them not visible/used simply because we decided to treat the discovered repository references as system repositories and then went into the framework to eliminate the system repositories from being actually used. Eventually, if we sweep broadly enough, the system repositories might as well not exist, circling us back to the question of what is a system repository and what for what is it used? It makes me uncomfortable that we are changing something without fully understanding it nor fully understanding the impact of our changes beyond our own use cases. |
Note that I would have less of an aversion to making behavior changes where we don't really know the consequences on downstream applications if it were possible for those applications to restore the current behavior quickly and easily, e.g., via a system property. Unfortunately we don't really know the original design intent and I don't think anyone will explain it to us. As such, we're kind of in a bind so it's probably best we err on the side of caution such that if someone complains, there is an immediate solution... |
7db1bb3
to
c2a2cbc
Compare
Currently if the user choose 'Check for Updates' the handler uses the ProvisioningUI (that uses RepositoryTracker internally) to choose the initial items to offer. The UpdateChecker (aka automatically search for updates) on the other hand uses a (potentially) different set of items. This now adds a new method that allow to pass the repository flags to be used when search for an update and delegate it on the callers side to use the Provisioning UI.
First I now adjusted the code so the caller can control what is "wanted", for platform I strongly believe we want the changed behavior because of the following reasons:
This is not a behavior change per se, it was simply inconsistent in the past where one functionality "Check For Updates" used
Every change breaks someones workflow for sure... nevertheless who ever "relied" on that behavior has always relied on something that was unreliable / unpredictable because as shown in the previous PR depends on:
depending on that a user can potentially see more or less updates, it could even happen that after one update the next time it gets even more. I believe that this is not any behavior one wants nor one should rely on.
At least I'm not aware of any more places but that's not surprising to me as the intend from the code and from the rare documentation is that I found is that it should not surface to the user, but in most other cases should be used.
That's because the planner obviously is not "the user" and requires that information, but the important part is that at this stage the update UIs are already selected so this is not different than what happens if I install new items (and I select contact all sites to find required items for example).
I have found for example this one that describes them as being added but never shown to the user, in this case it is a composite repository that has showed up suddenly, the issue also describes that a repository even permanently can mark it self as "system" or you can mark them inside the repository manager.
I now added |
c2a2cbc
to
7c7b2e8
Compare
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.
Thank you for the detailed and insightful analysis! Your comment about installing new things "at this stage the update UIs are already selected" is an interesting and relevant point.
I wonder why API tooling didn't report any issue here? See #514. |
Currently if the user choose 'Check for Updates' the handler uses the ProvisioningUI (that uses RepositoryTracker internally) to choose the initial items to offer. The UpdateChecker (aka automatically search for updates) on the other hand uses a (potentially) different set of items.
This now first fetches the RepositoryTracker service and if there use that to query for the repositories, if not there it uses only NON_SYSTEM repositories (what is the default in
RepositoryTracker#metadataRepositoryFlags)