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

blackmagic: 1.6.1 -> unstable-2019-08-13 #66469

Merged
merged 1 commit into from Aug 14, 2019

Conversation

@emilazy
Copy link
Contributor

commented Aug 11, 2019

I updated the version number to include the commit ID as upstream
apparently doesn't plan on making another stable release:

blacksphere/blackmagic#486

This is my first nixpkgs contribution, so apologies if I've messed
anything up! I had to switch to a newer gcc-arm-embedded to get it
to compile properly.

Motivation for this change
Things done
  • 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.
Notify maintainers

cc @pjones

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch 2 times, most recently from 296323f to e5fcb21 Aug 11, 2019

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from e5fcb21 to ca1cd04 Aug 12, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from ca1cd04 to 1ca1b7f Aug 12, 2019

@thoughtpolice

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

Other than that, this looks good to me!

@thoughtpolice

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@GrahamcOfBorg build blackmagic

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from 1ca1b7f to 07705c1 Aug 12, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@thoughtpolice This is what's in the manual

If a package is not a release but a commit from a repository, then the version part of the name must be the date of that (fetched) commit. The date must be in "YYYY-MM-DD" format. Also append "unstable" to the name - e.g., "pkgname-unstable-2014-09-23".

https://nixos.org/nixpkgs/manual/#sec-package-naming

For consistency for the tree I've been trying to have version be unstable-YYYY-MM-DD since the rule hasn't been updated for pname.

Normally, I try to keep the latest stable version number included in the version string, as well as the number of commits since that tag, and the git hash, rather than using the date. Nix will look at version in order to determine if e.g. nix-env -u needs to upgrade a package. (Check out builtins.compareVersions for more.)

I'll have to check this again, but last time I discussed this with someone there wasn't an issue with nix-env and this versioning.

@thoughtpolice

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

Ah, I see. I wasn't aware we actually had recommendations for this (personally I find dates a little less informative than the actual git-based string, and that's what I've been using for a long time, while being somewhat nicer for a user to tell -- but that's neither here nor there.) I retract my complaint, then!

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

Ah, I see. I wasn't aware we actually had recommendations for this (personally I find dates a little less informative than the actual git-based string, and that's what I've been using for a long time, while being somewhat nicer for a user to tell -- but that's neither here nor there.) I retract my complaint, then!

TBH we should change these recommendations to something a little bit better, IIRC all packages that use this versioning are ignored by repology.

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from 07705c1 to 9ad40f1 Aug 12, 2019

@emilazy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

Okay, I swapped the version back to the date format. I left out the -unstable from the name since upstream seems to consider the master branch to be stable and isn't planning on doing more versioned releases. Let me know if there's anything else I need to do to get this merged ^^

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

I left out the -unstable from the name since upstream seems to consider the master branch to be stable and isn't planning on doing more versioned releases.

unstable should be in the version, it's in the guideline if referenced.

@emilazy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

Okay, I bumped the commit and added the unstable- prefix. For what it's worth, the version = "YYYY-MM-DD" form seems to dominate in nixpkgs, although there's probably a fair number of packages that use that versioning scheme for stable releases, and I personally think it makes the most sense when considering packages for which a version from VCS isn't necessarily viewed as inherently unstable:

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"\d{4}-\d{1,2}-\d{1,2}"' | wc -l
876

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"\d{4}\.\d{1,2}\.\d{1,2}"' | wc -l
170

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"unstable-\d{4}-\d{1,2}-\d{1,2}"' | wc -l
127

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from 9ad40f1 to 54eef5c Aug 14, 2019

@emilazy emilazy force-pushed the emilazy:update-blackmagic branch from 54eef5c to d792c84 Aug 14, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Okay, I bumped the commit and added the unstable- prefix. For what it's worth, the version = "YYYY-MM-DD" form seems to dominate in nixpkgs, although there's probably a fair number of packages that use that versioning scheme for stable releases, and I personally think it makes the most sense when considering packages for which a version from VCS isn't necessarily viewed as inherently unstable:

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"\d{4}-\d{1,2}-\d{1,2}"' | wc -l
876

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"\d{4}\.\d{1,2}\.\d{1,2}"' | wc -l
170

[emily@yukari:~/etc/nixos/extern/nixpkgs]$ rg 'version *= *"unstable-\d{4}-\d{1,2}-\d{1,2}"' | wc -l
127

It's likely you'll find more entries in name for unstable along with the date in version since pname just happened recently.

@worldofpeace worldofpeace changed the title blackmagic: 1.6.1 -> 1.6.1-311-gfbf1963 blackmagic: 1.6.1 -> unstable-2019-08-13 Aug 14, 2019

@worldofpeace worldofpeace merged commit 24870b4 into NixOS:master Aug 14, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Thanks for contributing @emilazy.

Apologies on the little speed bump around version, I hope I explained everything properly.

@emilazy emilazy deleted the emilazy:update-blackmagic branch Aug 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.