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

Recursive dependency updates can result in breaking otherwise working dependencies #2264

Closed
2 tasks done
ckoopmann opened this issue Jul 11, 2022 · 3 comments
Closed
2 tasks done
Labels
T-bug Type: bug

Comments

@ckoopmann
Copy link
Contributor

ckoopmann commented Jul 11, 2022

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (f016135 2022-07-04T00:15:02.930499Z)

What command(s) is the bug in?

forge update

Operating System

macOS (Intel)

Describe the bug

Current Behaviour

When updating a dependency using forge update, all nested dependencies are updated to the latest version (if these nested dependencies are also managed using foundry / git submodule).

This can result in the installed library being in a state different from it's own source repository (which will be marked as COMMITHASH-dirty by git). In case the latest version of the nested dependency is not backwards compatible this can lead to the installed version of the package breaking. I created a sample repo that illustrates that issue here.

In some situations this might render the forge update command unusable since the user wont be able to upgrade a dependency without breaking it.

Even when it doesn't break the dependency it can lead to subtle differences in its behaviour, which might be even more dangerous.

Expected behaviour

The user should have the option to update the dependency without recursively updating nested dependencies beyond changes in the depencies repo itself.

Currently the only way for the user to achieve this is to manually reinstall the package using forge remove and forge install. It would be more ergonomic to let the user disable / enable recursive updates using a flag.

Also one might want to revisit the decision (if it was a conscious one) to have recursive updates by default since it results in behaviour that many users might find unexpected.

Possible solution

Remove the --recursive flag here entirely or conditionally based on user input.

@ckoopmann ckoopmann added the T-bug Type: bug label Jul 11, 2022
@gakonst
Copy link
Member

gakonst commented Jul 11, 2022

Good catch - I introduced this bug many months ago here, should be fine to remove the recursive flag I think.

@sambacha
Copy link
Contributor

you should just be doing

git submodule foreach git reset --hard HEAD
git submodule update --remote --rebase lib/

@onbjerg
Copy link
Member

onbjerg commented Jul 14, 2022

Fixed in #2274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

No branches or pull requests

4 participants