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

bonfire: init at 0.0.7 #29813

Merged
merged 1 commit into from Oct 4, 2017
Merged

bonfire: init at 0.0.7 #29813

merged 1 commit into from Oct 4, 2017

Conversation

womfoo
Copy link
Member

@womfoo womfoo commented Sep 26, 2017

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@womfoo womfoo mentioned this pull request Sep 26, 2017
Closed
vyp
vyp reviewed Sep 27, 2017
View reviewed changes
pkgs/tools/misc/bonfire/default.nix Outdated
with python3Packages;

buildPythonApplication rec {
version = "0.0.7-20170119";
Copy link
Member

Choose a reason for hiding this comment

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

In this case version should just be 20170119, unless this versioning scheme is recommended somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for reviewing @vyp . Rebased changes with just 20170119.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it should be 2017-01-19: https://nixos.org/nixpkgs/manual/#sec-package-naming

@orivej
Copy link
Contributor

orivej commented Oct 1, 2017 •
edited

@womfoo The package installs some extraneous files:

$ tree result/
├── AUTHORS.rst
├── bin
│   └── bonfire
…
├── LICENSE.txt
├── nix-support
│   └── propagated-native-build-inputs
├── README.rst
└── requirements.txt

Can you fix that?

@womfoo
Copy link
Member Author

womfoo commented Oct 4, 2017

@orivej just rebased the changes to remove extraneous files.

I also tried updating the version to 2017-01-19 and unstable-2017-01-19 but pip fails with these.

/nix/store/9b4d3vb92z7b4ydxdk1zg9slvcfdgaxm-python3.6-bootstrapped-pip-9.0.1/lib/python3.6/site-packages/setuptools/dist.py:360: UserWarning: The version specified ('unstable-2017-01-19') is an invalid version, this may not work as expected with newer versions of setuptools, pip, and PyPI. Please see PEP 440 for more details.
  "details." % self.metadata.version

@vyp
Copy link
Member

vyp commented Oct 4, 2017

@womfoo try putting "unstable" in pname so that pip gets the date as the version:

version = "2017-01-19";
pname = "bonfire-unstable";

@vyp
Copy link
Member

vyp commented Oct 4, 2017

But that would mean repo for src wouldn't work. Just put "unstable" in name then: name = "${pname}-unstable-${version}";.

@womfoo
Copy link
Member Author

womfoo commented Oct 4, 2017

Moving "unstable" in the version did fix it. Thanks @vyp

@orivej orivej merged commit 7c2b617 into NixOS:master Oct 4, 2017
@womfoo womfoo deleted the init/bonfire-0.0.7 branch February 3, 2018 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orivej orivej orivej left review comments

@vyp vyp vyp left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@womfoo @orivej @vyp