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

gruntfile: clean collection after unzipping #759

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

kiike
Copy link
Contributor

@kiike kiike commented Jul 10, 2024

During the packaging work for Nix, we noticed that the wget task would always try downloading the collections file, even though we downloaded it to the cache location. After some digging, it looks like cleaning the downloaded file before the download will just do that.

If we run the clean:collection task after unzipping, we allow downloading the file manually into the cache directory so that the unzip task doesn't try downloading it again. That is especially beneficial on NixOS, as the dependencies should be cached before running the build.

@kiike kiike force-pushed the pr/clean_collection_after_unzipping branch from bb14870 to 92e4520 Compare July 10, 2024 20:16
Co-authored-by: Alberto Merino <amerinor01@gmail.com>
Co-authored-by: Enric Morales <me@enric.me>
Co-authored-by: Roland Coeurjoly <rolandcoeurjoly@gmail.com>

If we run the `clean:collection` task after unzipping, we allow
downloading the file manually into the cache directory so that the unzip
task doesn't try downloading it again.
@kiike kiike force-pushed the pr/clean_collection_after_unzipping branch from 92e4520 to 62b0a63 Compare July 10, 2024 20:16
@cavearr
Copy link
Member

cavearr commented Jul 10, 2024

Great improvements for the packager , thanks!

In other way i'll try to try it on OSX , but i have errors (i'm novice with nix), i do this steps:

  1. Clone nixpkg repo
  2. checkout fern/icestudio
  3. Go to icestudio dir
  4. Excecute nix-build package.nix

but i receive the error error: cannot evaluate a function that has an argument without a value ('fetchurl')
Nix attempted to evaluate a function as a top level expression; in
this case it must have its arguments supplied either by default
values, or passed explicitly with '--arg' or '--argstr'. See
https://nixos.org/manual/nix/stable/language/constructs.html#functions.
at /Users/carl/work/nixpkgs/pkgs/by-name/ic/icestudio/package.nix:1:69:
1| { lib, stdenv, fetchFromGitHub, buildNpmPackage, nwjs, makeWrapper, fetchurl
| ^
2| , python3, python3Packages, }:

I'm asking you if there is a beginners guide or some documentation that you recomended.

@cavearr cavearr merged commit 08238da into FPGAwars:develop Jul 10, 2024
3 of 4 checks passed
@kiike
Copy link
Contributor Author

kiike commented Jul 10, 2024

Thanks for wanting to try it!

Clone nixpkg repo
checkout fern/icestudio
Go to icestudio dir
Excecute nix-build package.nix

The last step is not what you need to run. From the root of the nixpkgs repo, and after checking out the fern/icestudio branch, run nix-build -A icestudio. This means "build the attribute icestudio" which, sure, is a package for us, but for the package manager is an attribute.

After a hopefully successful build, a result directory will appear. You can then run ./result/bin/icestudio.

For documentation i'd check out nix.dev, like https://nix.dev/tutorials/packaging-existing-software.html#your-first-package.

Regarding this PR, it actually contains an error due to my misunderstanding of the clean:collection task. Although it says that it removes the previously downloaded collection, it removes both the downloaded collection and the unzipped collection, so the build will not be correct. May I ask to reopen this PR? I'd like to add a fix. Thanks!

@cavearr
Copy link
Member

cavearr commented Jul 10, 2024

The Merge is reverted!

@cavearr
Copy link
Member

cavearr commented Jul 10, 2024

And dont worry was my fault, i'm not remember well what "clean collections" do and i don't check in deep.

@cavearr
Copy link
Member

cavearr commented Jul 11, 2024

Hi! i'm trying to build in osx, but several errors appears, one of them and i think the first to start (others, i think are promoted by this one) in nixpkgs osx is not supported by nwjs pkg:

  error: Package ‘nwjs-0.88.0’ in /Users/carl/work/nixpkgs/pkgs/development/tools/nwjs/default.nix:141 is not available on the requested hostPlatform:
         hostPlatform.config = "aarch64-apple-darwin"
         package.meta.platforms = [
           "i686-linux"
           "x86_64-linux"
         ]

i'll try to add support to this!

@kiike
Copy link
Contributor Author

kiike commented Jul 11, 2024

Thank you so much for trying it! We'll definitely use your help I think 😄!

@cavearr
Copy link
Member

cavearr commented Jul 11, 2024

Hello @kiike! i have managed to package in nix nwjs for osx!!! Tomorrow I'll try to pack icestudio! I'm learning a lot through the process with nwjs. I feel more comfortable with nix at every step :)

Captura de pantalla 2024-07-11 a las 22 41 59

@kiike
Copy link
Contributor Author

kiike commented Jul 12, 2024 via email

@cavearr
Copy link
Member

cavearr commented Jul 14, 2024

Hi @kiike ! 'm forking nixpkgs and launch the PR for nwjs support.

Then i'm trying to checkout in my fork to fern/icestudio but i don't find it, you move it? in wich branch is icestudio now? thanks!!

@kiike
Copy link
Contributor Author

kiike commented Jul 15, 2024

Hey @cavearr! Nice! I'm excited you are taking these steps 👍! The branch is still active, but it lives in @jackleightcap's fork. So just add his remote. https://github.com/jleightcap/nixpkgs/ branch: fern/icestudio.

@cavearr
Copy link
Member

cavearr commented Jul 15, 2024

Ou!! sorry!! i'm fork from your PR and i not show it sorry! i have a PR for nwjs to the nixpkgs yesterday , in the afternoon or tomorror i'll do it to your fork (nwjs and icestudio).

Thanks again for your patience!

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.

2 participants