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

Darc does not update packages which were added in an existing PR #3562

Open
ericstj opened this issue May 29, 2024 · 3 comments
Open

Darc does not update packages which were added in an existing PR #3562

ericstj opened this issue May 29, 2024 · 3 comments

Comments

@ericstj
Copy link
Member

ericstj commented May 29, 2024

See dotnet/efcore@79197f7

I had added two packages to Version.Details.xml. Darc then modified the PR with a new version of runtime but it didn't update the packages I added.

Probably Darc is looking at the target branch's file rather than the PR branch's file.

@riarenas riarenas transferred this issue from dotnet/arcade May 29, 2024
@mmitche
Copy link
Member

mmitche commented May 29, 2024

Probably Darc is looking at the target branch's file rather than the PR branch's file.

Yes, Maestro is reading from the target branch initially when creating the branch, and either it keeps looking at the target branch, or it has a list of previously updated packages and only uses those for future updates. Can't remember which.

We can probably make a partial fix by reading dependencies from the PR branch if an existing PR is already open. A 'full' fix would be more difficult. A common scenario that happens is that a PR is opened for updates, someone adds a new dependency in a different PR, and both PRs merge. Often they don't conflict so you can get incoherency that way. Maestro would need to be aware of the PR target branch's current state and ensure that the dependency set wasn't changed. If it was, then it needs to restart the update process, pulling in a new build.

@premun
Copy link
Member

premun commented Aug 7, 2024

We made a related small change that reads the versions from the PR head branch and I think it's possible to make it behave this way. The fix itself can be quite simple but I am worried about not covering scenarios such as people removing a dependency in the PR or similar which makes this a bit more complicated of a change. But we will see if we can do this one.

@mmitche
Copy link
Member

mmitche commented Aug 7, 2024

There are for sure a number of corner cases here in order to get things perfect. Reading from the PR branch is an incremental improvement and still worth it IMO.

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

No branches or pull requests

3 participants