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.

Sign up for GitHub

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

Jump to bottom

Simplify Pkg2.resolve #3615

Merged
merged 1 commit into from
Jul 5, 2013
Merged

Simplify Pkg2.resolve #3615

merged 1 commit into from
Jul 5, 2013

Conversation

Keno
Copy link
Member

@Keno Keno commented Jul 3, 2013

@Keno
Copy link
Member Author

Keno commented Jul 5, 2013

@StefanKarpinski updated. Not sure where to put the dependencies and alldependencies functions. I'm tempted to put them in Query because diff needs them, but Query already has one that does something different, so I put then in Read for now. Let me know where you want them.

@StefanKarpinski
Copy link
Sponsor Member

Cool. The names of these functions could use some bikeshedding, along with the overall API. This is fine for now.

StefanKarpinski added a commit that referenced this pull request Jul 5, 2013
@StefanKarpinski
Merge pull request #3615 from JuliaLang/kf/pkg2
04cf140 
Simplify Pkg2.resolve
@StefanKarpinski StefanKarpinski merged commit 04cf140 into master Jul 5, 2013
StefanKarpinski
StefanKarpinski reviewed Jul 5, 2013
View reviewed changes
base/pkg2/read.jl
@@ -123,4 +123,24 @@ function free(inst::Dict=installed())
return pkgs
end

function dependencies(pkg::String, avail::Dict=available(),free::Dict=free(installed(avail)),fix::Dict=fixed(avail,installed(avail)))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This signature is problematic: if you use the defaults, then installed is called twice. This should probably only exist without defaults.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, sounds good. I wasn't sure what to do, since I didn't want to introduce an extra installed parameter if it's never used. I'll remove the defaults.

@StefanKarpinski
Copy link
Sponsor Member

I've reverted this because it doesn't seem to work at all. I can't do Pkg2.resolve() at all without getting complaints that every package that's present is neither free nor fixed, which is clearly not the case.

@Keno
Copy link
Member Author

Keno commented Jul 5, 2013

Hmm, interesting. It worked for me. Let me see what the issue is.

@Keno
Copy link
Member Author

Keno commented Jul 5, 2013

Could you let me know what command you were running that's failing?

@Keno
Copy link
Member Author

Keno commented Jul 11, 2013

@StefanKarpinski can we try to get this one resolved, so I can rebase my other changes to Pkg2 and we can discuss those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Keno @StefanKarpinski