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

Paket Update is adding dependencies to paket.dependencies if specified in paket.references but not in paket.dependencies #779

Closed
cjbhaines opened this issue Apr 15, 2015 · 25 comments · Fixed by #781

Comments

@cjbhaines
Copy link
Contributor

Paket version 0.41.3.

If you have a package referenced in paket.references and it doesn't exist in paket.dependencies it adds it without any version constraints. As far as I understand it, the paket.dependencies file should never be modified by Paket itself unless you do an add?

@forki
Copy link
Member

forki commented Apr 15, 2015

Actually this is a feature. Requested by @haf and @agross ;-)

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Really? I don't remember requesting that. Are you sure we're talking about references and dependencies?

@cjbhaines
Copy link
Contributor Author

Yes, it definitely modifies my paket.dependencies file. That sounds extremely dangerous to me as it adds them without constraints. Can this be made opt in with a flag please as I'm fairly sure that most users will not want that enabled? This has broken a lot of my builds (it's partly bad configuration from us as we shouldn't reference stuff in paket.references that isn't in paket.dependencies so we are working on that).

@forki
Copy link
Member

forki commented Apr 15, 2015

#435 (comment) is what was suggested. @agross you were in that discussion so i thought you agreed ;-)

@forki
Copy link
Member

forki commented Apr 15, 2015

ok. let's discuss how to make this configurable.

@cjbhaines
Copy link
Contributor Author

There is a secondary problem here that it moves my sources around.

Original paket.dependencies:

source myinternalrepo
package..
package...

source officalrepo
package...

After update:

source myinternalrepo
source officalrepo
package..
package...
package...

@forki
Copy link
Member

forki commented Apr 15, 2015

Please create a second issue

@cjbhaines
Copy link
Contributor Author

#780

@agross
Copy link
Contributor

agross commented Apr 15, 2015

It seems I didn't read @haf's suggestion entirely or didn't understand its consequences.

From my point of view, the only one who will modify paket.dependencies is either the user or paket add.

Alex

Alexander Groß
Tiny phone, tiny mail

On Wed, Apr 15, 2015 at 12:20 PM, Steffen Forkmann
[email protected] wrote:

Please create a second issue

Reply to this email directly or view it on GitHub:
#779 (comment)

@forki
Copy link
Member

forki commented Apr 15, 2015

Yeah it's basically like add. Consider the 1000 csproj-files solution from @Thorium - I don't really see a way to make this work without that. But I agree we should make that configureable and maybe even disabled on default.

@cjbhaines
Copy link
Contributor Author

1 for disabled by default :)

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Sorry, I don't see how automatically modifying the depfile "helps" with 1000 projects. You would need to modify 1000 references nonetheless.

@forki
Copy link
Member

forki commented Apr 15, 2015

Nope what I meant is:

You have 1000 projects and you are working in 1. Now you want to use a nuget. How would you install it?

@agross
Copy link
Contributor

agross commented Apr 15, 2015

paket add nuget Foo project Bar?

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Or manually (what I would probably do):

  • Edit depfile adding nuget Foo
  • Edit paket.references for Bar project
  • paket install

@cjbhaines
Copy link
Contributor Author

I usually go down the manual route as well.

Also, would it make sense to fail the update in the case where you have something in references and not in dependencies? Obviously, if you enable the upcoming config option to fix the dependencies file instead it wouldn't fail.

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Yes, it should definitely fail when you paket.references something that's not in paket.dependencies!

@forki
Copy link
Member

forki commented Apr 15, 2015

what about this:

We could you ask the user during install and let it fail if he doesn't want it. I think this could make everybody happy.

/cc @haf

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Not sure I'd be happy with that. I'd love the learn why @haf thinks adding deps automagically (even with explicit consent) is useful to him.

@agross
Copy link
Contributor

agross commented Apr 15, 2015

Background: I want to limit the paths that you can take to add a dep. We currently have three:

  • manual edit && paket install
  • paket add nuget
  • automagically adding from references

While 1 is absolutely crucial, and 2 is convenient, I feel that 3 is just a tad too magical for my taste.

@cjbhaines
Copy link
Contributor Author

@agross, totally agree

@haf
Copy link
Member

haf commented Apr 15, 2015

From the referenced issue, it's only point no 4 from #435 (comment) which this thread #779 is about - i.e. how should Paket react to adding new references through paket.references, rather than through paket.dependencies or paket add.

From this comment it's only the first four paragraphs under the install heading I really care strongly about, not the fifth, which this thread is about.

I'd be open to this change.

The reason I created that thread in the first place is that I wanted paket install to 'just download and setup the fsproj files all-in-one' while update takes care to actually do version-levelling properly. Which wasn't the case before.

After having used Paket for a long(-ish) time now, I can say that my own workflow is a combination of paket-add and editing paket.dependencies when I actually want to add a new nuget.

forki added a commit that referenced this issue Apr 15, 2015
if specified in paket.references but not in paket.dependencies

closes #779
@forki
Copy link
Member

forki commented Apr 15, 2015

so #781?

@forki
Copy link
Member

forki commented Apr 15, 2015

please retry with 0.42

@cjbhaines
Copy link
Contributor Author

Yep, all good now. Thanks for the quick response guys, I really appreciate it!

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 a pull request may close this issue.

4 participants