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

cargo-generate: init at 0.3.0 #64245

Merged
merged 1 commit into from Jul 24, 2019
Merged

Conversation

turboMaCk
Copy link
Member

@turboMaCk turboMaCk commented Jul 3, 2019

Motivation for this change

Usefull missing tool

Things done

Expression added

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

$ nix-review pr 64245 passes on NixOS
diff LGTM
binary works

[2 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/64245
1 package were build:
cargo-generate
./results/cargo-generate/bin/cargo-generate --help
cargo-generate 0.3.0
Ashley Williams <ashley666ashley@gmail.com>
Generate a new Cargo project from a given template
...

@xrelkd
Copy link
Contributor

xrelkd commented Jul 7, 2019

@GrahamcOfBorg build cargo-generate

@turboMaCk
Copy link
Member Author

@xrelkd I think there is probably an issue with macos build still. I've borrowed my mac to the girlfriend who has her on repair (keyboard :/) but I tried to build this quickly on that machine and ran into some issues with linking C libs. I will have that machine back soon enough so I think it makes sense to wait with merge for now after I make sure it works on darwin as well.

@turboMaCk
Copy link
Member Author

@xrelkd build on darwin is now fixed. Do you want me to rebase this against current master?

@turboMaCk turboMaCk force-pushed the tools-rust/cargo-generate branch 2 times, most recently from 45eba4e to 71bb45f Compare July 22, 2019 21:39
@turboMaCk turboMaCk requested a review from FRidh as a code owner July 22, 2019 21:39
@turboMaCk turboMaCk force-pushed the tools-rust/cargo-generate branch 2 times, most recently from 45eba4e to 925faca Compare July 22, 2019 21:42
@xrelkd
Copy link
Contributor

xrelkd commented Jul 23, 2019

Not sure.
BTW, I do not have the privilege of merging pull request.
Hi, @marsam Could you review this and give @turboMaCk some suggestions? Thank you.

# Tests depend on a valid git configuration.
# Rather than doing some crazy hack and adding additional dependencies
# we simply avoid them completely.
doCheck = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe try

preCheck = ''
  export GIT_COMMITTER_NAME=nixbld
  export EMAIL=nixbld@localhost
  export GIT_COMMITTER_DATE=$SOURCE_DATE_EPOCH
'';

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the whole test suit will do quite some side-effects. Just adding this produces following error during build

---- basics::it_substitutes_projectname_in_cargo_toml stdout ----
thread 'basics::it_substitutes_projectname_in_cargo_toml' panicked at 'Unexpected failure.
code-128
stderr=```fatal: invalid date format: 1

command="git" "commit" "--message" "initial commit"
code=128
stdout=``````
stderr=```fatal: invalid date format: 1

', /build/cargo-generate-0.3.0-vendor/assert_cmd/src/assert.rs:140:13

I'm not sure how to fix this but I can try to figure it out. Anyway I would expect there will be more issues along the way and am not sure if it's worth trying to fix all of those.

pkgs/development/tools/rust/cargo-generate/default.nix Outdated Show resolved Hide resolved
@marsam
Copy link
Contributor

marsam commented Jul 24, 2019

@turboMaCk sorry for the delay, LGTM. before I merge it, would you mind squashing the commits?

@turboMaCk turboMaCk force-pushed the tools-rust/cargo-generate branch 2 times, most recently from e945128 to 94335e1 Compare July 24, 2019 19:43
@turboMaCk
Copy link
Member Author

@marsam don't worry it was wip most of the time (waiting for me to find a time to fix darwin). Now it's squashed. there is still doCheck = false. If you want me to try to make tests works before merge just say so. Otherwise feel free to merge.

@marsam marsam merged commit b5f5c97 into NixOS:master Jul 24, 2019
@marsam
Copy link
Contributor

marsam commented Jul 24, 2019

IMHO I think it's always beneficial to enable the tests; but in some cases, like this one, is a bit complicated to setup, and it shouldn't hold its merge.
Built and tested locally on NixOS and Darwin. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants