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

Local paket.exe cache #1400

Merged
merged 10 commits into from
Jan 19, 2016
Merged

Local paket.exe cache #1400

merged 10 commits into from
Jan 19, 2016

Conversation

kerams
Copy link
Contributor

@kerams kerams commented Jan 16, 2016

Implements #1398. @xavierzwirtz

Do we need some mechanism for cache clearing?

@kerams
Copy link
Contributor Author

kerams commented Jan 16, 2016

I'll update the docs later today.

@forki
Copy link
Member

forki commented Jan 16, 2016

If it goes into nuget cache, then it will be cleaned with "paket clear-cache"

@kerams
Copy link
Contributor Author

kerams commented Jan 16, 2016

It does not right now. Do you want it to be cached there? It seems to me these are 2 slightly different use cases, i.e. you may want clear the nuget cache, but at the same time you may want to retain paket.exe cache.

@forki
Copy link
Member

forki commented Jan 16, 2016

In v3 we have our own cache folder. Maybe that would do
On Jan 16, 2016 2:47 PM, "kerams" notifications@github.com wrote:

It does not right now. Do you want it to be cached there? It seems to me
these are 2 slightly different use cases, i.e. you may want clear the nuget
cache, but at the same time you may want to retain paket.exe cache.


Reply to this email directly or view it on GitHub
#1400 (comment).

@kerams
Copy link
Contributor Author

kerams commented Jan 16, 2016

Alright, let me know how you want to proceed.

@forki
Copy link
Member

forki commented Jan 16, 2016

I think @xavierzwirtz should talk a bit about the use case so that we can optimize for that. I don't really care of the cache location

@xavierzwirtz
Copy link
Contributor

I am using paket on multiple projects, using the ProjectScaffold style build.cmd that auto updates paket. When I am working from a bandwidth constrained network, each project I build typically has to redownload the latest paket version. This can take upwards of ten minutes per.

@forki
Copy link
Member

forki commented Jan 16, 2016

ok so it's "just" to optimize download speed on multiple projects. I think we should go with the nuget cache then.

@kerams
Copy link
Contributor Author

kerams commented Jan 17, 2016

OK, last question. Are you fine with caching being the default or do you want to make it opt-in?

@forki
Copy link
Member

forki commented Jan 17, 2016

I think it should always cache and only -f flag should skip the cache. That
would be consistent with paket.exe
On Jan 17, 2016 9:21 AM, "kerams" notifications@github.com wrote:

OK, last question. Are you fine with caching being the default or do you
want to make it opt-in?


Reply to this email directly or view it on GitHub
#1400 (comment).

@kerams
Copy link
Contributor Author

kerams commented Jan 17, 2016

👍 I think it's ready to roll.

@forki
Copy link
Member

forki commented Jan 17, 2016

@xavierzwirtz can you please give it a try before I release it?

@mexx
Copy link
Member

mexx commented Jan 17, 2016

Shouldn't the default be the other way around, like it now is?
I mean, currently there are many users of Paket having the bootstrapper in their build script running it on some build servers, right? So in order to be sure always to have the latest stable version of Paket in the script they would have to clean the cache?

I vote for adding --cache option.

@forki
Copy link
Member

forki commented Jan 17, 2016

I meant the following:

  • check which version is latest
  • if latest is on folder take that one
  • if latest is in cache then take that one
  • otherwise download latest to cache and take that one
  • if we can't retrieve the latest version no. then we take latest cached
    version
  • if -f flag is set then we download and overwrite the cached version
    On Jan 17, 2016 2:23 PM, "Max Malook" notifications@github.com wrote:

Shouldn't the default be the other way around, like it now is?
I mean, currently there are many users of Paket having the bootstrapper in
their build script running it on some build servers, right? So in order to
be sure always to have the latest stable version of Paket in the script
they would have to clean the cache?

I vote for adding --cache option.


Reply to this email directly or view it on GitHub
#1400 (comment).

@kerams
Copy link
Contributor Author

kerams commented Jan 17, 2016

@mexx, just like forki says. The latest version is always determined by what's on github/nuget, not by looking in the cache.

  • if we can't retrieve the latest version no. then we take latest cached
    version

This is not currently implemented though and I'm not sure it's really that important to be honest :).

@mexx
Copy link
Member

mexx commented Jan 18, 2016

You are right guys, I had a second look on the code, not only the PR changes, and it works as you describe. All fine.

About the

This is not currently implemented though and I'm not sure it's really that important to be honest :).

I think it actually is, it would allow to build in a complete disconnected environment.
Imagine I have all of the dependencies in my local cache and sit in an airplane, the only thing what would prevent a build would be the inability to find out whether the cached version is the latest one, right?

@forki
Copy link
Member

forki commented Jan 18, 2016

Yes I think we should add this, too
On Jan 18, 2016 2:41 AM, "Max Malook" notifications@github.com wrote:

You are right guys, I had a second look on the code, not only the PR
changes, and it works as you describe. All fine.

About the

This is not currently implemented though and I'm not sure it's really that
important to be honest :).

I think it actually is, it would allow to build in a complete disconnected
environment.
Imagine I have all of the dependencies in my local cache and sit in an
airplane, the only thing what would prevent a build would be the inability
to find out whether the cached version is the latest one, right?


Reply to this email directly or view it on GitHub
#1400 (comment).

@kerams
Copy link
Contributor Author

kerams commented Jan 18, 2016

I see your point. I'll check whether it would be possible to implement without redoing the work done so far.

@xavierzwirtz
Copy link
Contributor

@forki Still need me to take a look at this?

I do like @mexx idea of not needing to be online to do a version check, that has bitten me before as well.

@forki
Copy link
Member

forki commented Jan 18, 2016

as I said: I think should take latest cached version when it can't connect
for version check

2016-01-18 18:09 GMT+01:00 Xavier Zwirtz notifications@github.com:

@forki https://github.com/forki Still need me to take a look at this?

I do like @mexx https://github.com/mexx idea of not needing to be
online to do a version check, that has bitten me before as well.


Reply to this email directly or view it on GitHub
#1400 (comment).

@kerams
Copy link
Contributor Author

kerams commented Jan 18, 2016

This is what I have now - if the online version check fails, but there is a newer version in the cache, we'll take that one. Unfortunately, as a side effect of the changes, you cannot downgrade from a prerelease version to the latest stable unless you specify the exact version. I don't yet know how to easily avoid this, so feel free to take a look if you can come up with something.

forki added a commit that referenced this pull request Jan 19, 2016
@forki forki merged commit f485ca9 into fsprojects:master Jan 19, 2016
@forki
Copy link
Member

forki commented Jan 19, 2016

awesome. Let's dogfood that thing for a while

@kerams kerams deleted the kerams-paket-exe-cache branch January 19, 2016 09:44
@xavierzwirtz
Copy link
Contributor

👍 This fixes my issue completely. Thanks for jumping on this @kerams and @forki!

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.

4 participants