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

Fix #744: Allow _checkouts dep to override pkg dep not in registry #775

Merged
merged 1 commit into from
Sep 5, 2015

Conversation

tsloughter
Copy link
Collaborator

Fix for #744 but won't make sense until after #760 is merged because it builds on it.

@ferd
Copy link
Collaborator

ferd commented Sep 4, 2015

I would probably prefer if we explicitly set the version value to something like checkout rather than undefined to keep it obvious that this is the only context where this should happen?

The one thing I'm not sure I get is why is the appinfo record requiring to get its deps inserted there wheras the source deps don't need to, and neither actually did before this patch. It seems like only the first and last clause should otherwise be required?

@tsloughter
Copy link
Collaborator Author

It can't be checkout, it is even before we discover it is in _checkouts that we are passing undefined as the version. That is simply the case where in rebar.config you don't put a version on the package, meaning for it to find and lock on the highest version available.

The deps are being added to app_info in the case of pkgs because we need to verify the package is in the registry anyway, so we may as well use the lookup that does this to get the deps and set them. In the case of a source dep we haven't yet fetched the dep so we can't set its dependencies at this point.

@ferd
Copy link
Collaborator

ferd commented Sep 5, 2015

Ah yeah I see, it was in pkg_to_app before. Got it.

ferd added a commit that referenced this pull request Sep 5, 2015
Fix #744: Allow _checkouts dep to override pkg dep not in registry
@ferd ferd merged commit 7de9e6b into erlang:master Sep 5, 2015
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

Successfully merging this pull request may close these issues.

None yet

2 participants