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

Handle rewrite map collisions #44

Closed
schomatis opened this issue Aug 13, 2018 · 3 comments · Fixed by #47
Closed

Handle rewrite map collisions #44

schomatis opened this issue Aug 13, 2018 · 3 comments · Fixed by #47
Assignees
Labels

Comments

@schomatis
Copy link
Collaborator

When building the rewrite map (buildRewriteMapping) new entries for the same DvcsImport will overwrite the old ones, so if a package.json explicitly declares a dependency A with version 2.0 but another dependency B (processed later) also has a dependency to A but with an older 1.0 version, this older version will get rewritten, the root package will be using the old version even though it explicitly declared the new one.

A first approach would be to prioritize dependencies in the package.json (over indirect dependencies), but I also have the doubt of why the map needs to go through the entire dependency tree to rewrite just the root package (I'm thinking of the gx install scenario, where the rewrite -post-install hook- will be called explicitly on every package separately).

@schomatis schomatis added the bug label Aug 13, 2018
@schomatis schomatis self-assigned this Aug 13, 2018
@schomatis
Copy link
Collaborator Author

I also have the doubt of why the map needs to go through the entire dependency tree to rewrite just the root package

Because packages like go-blockservice import dependencies (e.g., go-cid) without declaring it in their package.json. Is that standard practice?

@Stebalien
Copy link
Collaborator

See #38 (comment) and the related discussion.

@schomatis
Copy link
Collaborator Author

schomatis commented Aug 15, 2018

Thanks for the reference. Performance issues aside, I think we should explicitly declare all the imports in package.json for pretty much all the reasons discussed there.

If that can't be fixed at the moment I still think buildRewriteMapping should give priority to the dependencies explicitly declared in package.json and don't overwrite them with transitive dependencies.

And as an extra, we could add a warning in gx-go install if we're rewriting a dependency that was not declared in package.json and was taking it from a transitive dependency (the downside is that I think gx install won't show it).

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

Successfully merging a pull request may close this issue.

2 participants