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

Include paket.lock file? #66

Closed
isaacabraham opened this issue Apr 26, 2018 · 18 comments
Closed

Include paket.lock file? #66

isaacabraham opened this issue Apr 26, 2018 · 18 comments

Comments

@isaacabraham
Copy link
Member

Given that we're now including a yarn.lock file, shouldn't we do the same with paket.lock? Not only will this increase the speed for the first build (important IMHO), but will also allow us to ensure stable releases.

@Krzysztof-Cieslak
Copy link
Member

Uhh, we don’t do that? Yeah, we definitely should, IMO

@theimowski
Copy link
Member

It's tricky given the options of template.
Have a look e.g. at paket.dependencies - There's a lot of conditionals based on the input parameters (Server, Remoting, Fulma).
If we were to include a paket.lock, we'd have to generate one for each permutation of the available options.

What could be done is restrict the dependency versions - more stable releases, but still compromising first build speed.

@isaacabraham
Copy link
Member Author

Hmmmm. This is going to get worse as well now that we're adding Azure dependencies in (AppInsights soon, and then Storage as well). The install process doesn't actually take that long, especially since install:none is set.

I suppose that the only alternative would be to not have any conditionals at all and just have a single lock file for everything - not a great solution though.

@theimowski
Copy link
Member

Yep I think the advantage of fast first build doesn't outgrow the cost of having unused dependencies in the project. Closing for now

@theimowski
Copy link
Member

reopening - it's getting more and more irritating, specially since stuff like AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#965 happens and one cannot have deterministic build when creating from given version of template...
@matthid suggested using groups for this, that could work but would require quite a lot of effort

@theimowski theimowski reopened this Jun 19, 2018
@supertom44
Copy link

I just came to create a new project from the template and got caught out by the above-linked error.

Is there a quick workaround while the issue is being resolved?

Cheers.

@theimowski
Copy link
Member

You can pin System.IdentityModel.Tokens.Jwt to a previous version, IIRC 5.2.2

@isaacabraham
Copy link
Member Author

isaacabraham commented Jun 24, 2018

@theimowski how about a compromise here - let's either:

  1. Create a paket lock for when the default options only are selected. Otherwise, it doesn't create one.

  2. OR we create a massive lock file that contains everything for all packages (this potentially introduces other problems though).

I demoed creating a SAFE app from scratch the other night and had to pre-generate the lock file because it simply took too long otherwise.

@forki
Copy link
Member

forki commented Jun 24, 2018 via email

@isaacabraham
Copy link
Member Author

OK - let's look to do that after #101 goes in.

@isaacabraham
Copy link
Member Author

I've just tested out with a clean safe template and included a ready-made paket.lock, it definitely helps a lot. The biggest slow downs that I saw now are (I think) compilation of the javascript side and also this "linking" process of the yarn dependencies. Is there any way that we can "precompute" that as well?

@forki
Copy link
Member

forki commented Jun 25, 2018 via email

@isaacabraham
Copy link
Member Author

The lock file is already there.

@isaacabraham
Copy link
Member Author

@forki this is what I'm referring to yarnpkg/yarn#1496

@forki
Copy link
Member

forki commented Jun 26, 2018 via email

@theimowski
Copy link
Member

#109 solves the issue partially, I'll keep it open until we have a consistent solution for all possible options

@theimowski
Copy link
Member

Please review #139

@theimowski
Copy link
Member

Should be resolved by #139 now

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

No branches or pull requests

5 participants