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

moon: v1.5.1 #229867

Merged
merged 2 commits into from
May 30, 2023
Merged

moon: v1.5.1 #229867

merged 2 commits into from
May 30, 2023

Conversation

flemzord
Copy link
Contributor

@flemzord flemzord commented May 4, 2023 •
edited
Loading

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 4, 2023
NobbZ
NobbZ requested changes May 5, 2023
View reviewed changes
Copy link
Contributor

@NobbZ NobbZ left a comment

Choose a reason for hiding this comment

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

  1. Please create another commit that creates you as a maintainer in maintainers/maintainer-list.nix
  2. Make that commit happen before your current one with message maintainers: add flemzord
  3. Add the package to pkgs/top-level/all-packages.nix
  4. This adding should happen in the same commit as creating the package expression
  5. The commit message should say moon: init at v1.4.0

So there should be 2 commits touching 3 files.

@NobbZ
Copy link
Contributor

NobbZ commented May 5, 2023

PS, I get an error trying to build (only parts of the output I think are relevant):

moon-v1.4.0>   running cd "/build/source/target/x86_64-unknown-linux-gnu/release/build/openssl-sys-ab01d5f7a87520dc/out/openssl-build/build/src" && AR="ar" ARFLAGS="" CC="/nix/store/b5mkki8bysygaf7a01jpj88zrxibkrv4-gcc-wrapper-12.2.0/bin/cc" RANLIB="ranlib" RANLIBFLAGS="" "perl" "./Configure" "--prefix=/build/source/target/x86_64-unknown-linux-gnu/release/build/openssl-sys-ab01d5f7a87520dc/out/openssl-build/install" "--openssldir=/usr/local/ssl" "no-dso" "no-shared" "no-ssl3" "no-unit-test" "no-comp" "no-zlib" "no-zlib-dynamic" "no-md2" "no-rc5" "no-weak-ssl-ciphers" "no-camellia" "no-idea" "no-seed" "linux-x86_64" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64"
moon-v1.4.0>   --- stderr
moon-v1.4.0>   thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', /build/moon-v1.4.0-vendor.tar.gz/openssl-src/src/lib.rs:507:39
moon-v1.4.0>   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
moon-v1.4.0> warning: build failed, waiting for other jobs to finish...

@NobbZ
Copy link
Contributor

NobbZ commented May 13, 2023 •
edited
Loading

@flemzord are you still interested in this getting merged? Please reply before May 21st or I will unwatch this PR.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 13, 2023
@NobbZ
Copy link
Contributor

NobbZ commented May 13, 2023

After your most recent updates, the mentioned build error remains.

@NobbZ
Copy link
Contributor

NobbZ commented May 13, 2023

Result of nixpkgs-review pr 229867 run on x86_64-linux 1

1 package failed to build:
  • moon

@flemzord flemzord changed the title moon: v1.4.0 moon: v1.5.1 May 13, 2023
@flemzord
Copy link
Contributor Author

I just pushed a new version and it works for me locally on my Mac.

@NobbZ
Copy link
Contributor

NobbZ commented May 14, 2023

Build on linux still fails for me, how did you build it there?

@dit7ya
Copy link
Member

dit7ya commented May 14, 2023

@flemzord I tried to package this some time ago and IIRC moon tried to bring its own nodejs toolchain (downloads from the internet) which do not work on NixOS since those nodejs binaries are not patched - making the package almost useless on NixOS. Has anything changed in that regard?

@flemzord
Copy link
Contributor Author

@dit7ya I am not a Moon developer only a user
Their tool is coded in rust and with a distribution in binary and via npm

On the NodeJS part, I think you're talking about proto? Which allows to manage dependencies?

@dit7ya
Copy link
Member

dit7ya commented May 14, 2023

I am talking about the nodejs binary that moon uses to do anything.

I just looked up and seems now they support the system node available on PATH.

https://moonrepo.dev/docs/setup-toolchain#configuring-nodejs

By default, the language will execute tasks using the global node binary available on PATH. While this works, we suggest using our toolchain that will download, install, and execute tasks with the same version of Node.js across all machines. To enable the toolchain, simply define the version with node.version.

@flemzord flemzord force-pushed the master branch 2 times, most recently from fcb2721 to 0081416 Compare May 14, 2023 08:11
@NobbZ
Copy link
Contributor

NobbZ commented May 14, 2023

Result of nixpkgs-review pr 229867 run on x86_64-linux 1

1 package built:
  • moon

@flemzord flemzord requested a review from dit7ya May 17, 2023 05:37
dit7ya
dit7ya suggested changes May 17, 2023
View reviewed changes
Copy link
Member

@dit7ya dit7ya left a comment

Choose a reason for hiding this comment

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

In general looks good - just some nitpicks. Also, welcome to nixpkgs :).

pkgs/development/tools/build-managers/moon/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/build-managers/moon/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/build-managers/moon/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/development/tools/build-managers/moon/default.nix Outdated Show resolved Hide resolved
@flemzord flemzord requested a review from dit7ya May 18, 2023 08:19
dit7ya
dit7ya approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@dit7ya dit7ya left a comment

Choose a reason for hiding this comment

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

LGTM.

pkgs/development/tools/build-managers/moon/default.nix Show resolved Hide resolved
@dit7ya dit7ya mentioned this pull request May 19, 2023
Closed
@putchar
Copy link
Contributor

putchar commented May 30, 2023

Result of nixpkgs-review pr 229867 run on x86_64-linux 1

1 package built:
  • moon

@NobbZ NobbZ merged commit 8a30776 into NixOS:master May 30, 2023
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dit7ya dit7ya dit7ya approved these changes

@NobbZ NobbZ NobbZ approved these changes

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@flemzord @NobbZ @dit7ya @putchar @Janik-Haag