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
Member

@emilazy emilazy 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:

blackmagic-debug/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

@worldofpeace
Copy link
Contributor

cc @matthewbauer

@thoughtpolice
Copy link
Member

Other than that, this looks good to me!

@thoughtpolice
Copy link
Member

@GrahamcOfBorg build blackmagic

@worldofpeace
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

emilazy 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
Copy link
Contributor

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
Copy link
Member Author

emilazy 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

@worldofpeace
Copy link
Contributor

worldofpeace 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
Copy link
Contributor

Thanks for contributing @emilazy.

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

@emilazy emilazy deleted the update-blackmagic branch August 14, 2019 00:20
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

4 participants