apitrace: 7.1 -> git #24829

binarin · 2017-04-11T15:20:12Z

Motivation for this change

After upgrade qapitrace have working "Buffers" tab where the data
can be inspected (it was always empty before).

There is no tags after 7.1, but I think that fixing pretty important
piece of functionality warrants an upgrade to current master tip.

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 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.

mention-bot · 2017-04-11T15:20:14Z

@binarin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nckx, @ocharles and @FRidh to be potential reviewers.

ocharles · 2017-04-11T15:33:12Z

I would certainly benefit from this in the work I do. Will see what the other reviewers think before merging.

nckx

Sure!

Mic92 · 2017-04-13T13:23:15Z

pkgs/applications/graphics/apitrace/default.nix

@@ -2,11 +2,11 @@

 stdenv.mkDerivation rec {
  name = "apitrace-${version}";
-  version = "7.1";
+  version = "7.1-git-20170411";


should be version = "unstable-20170411"; per convention.

It's even better to use pre-20170411, since then it will preceed version 7.1 in the lexicographical ordering used by nix-env -u (i.e. 7.1-pre-20170411 will upgrade to 7.1, whereas 7.1-unstable-... will not).

I'm a bit confused about @Mic92 suggestion to use just the unstable-20170411 version, without any reference to the current stable version. What are the reasoning behind this?

I think that 7.1-unstable-... not upgrading to 7.1, but upgrading to 7.2 (whenever it'll be released) is the desired outcome. So the 7.1-unstable-20170411 should be OK, isn't it?

This was taken from our package naming convention as it would refer to an unreleased package after the 7.1 release: https://nixos.org/nixpkgs/manual/#sec-package-naming adding 7.1 or 7.1-pre would indicate the release 7.1 or a prelease of it, which this commit is not. We would take the convention suggested by @edolstra, if a 7.1 release would follow afterwards.

pre is a nice tool, but we would want 7.2-pre... (the point is to order between 7.1 and 7.2).

But, you would have to be sure about the release scheme. Otherwise there could be a 7.1.1.

I think a git describe-like version scheme works best for unstable releases of upstream projects that do releases. (I think the point about using date as version was for projects that didn't do releases at all.) Using pre assumes you know the next version number. Using git describe-like doesn't assume anything.

I've checked the history of apitrace releases, it looks like only 2-component versions are used. So the options are:

7.2-pre-20170414

7.1-363-ge3509be1 from git describe

I like the second one more. If it's OK, I'll use it and also I'll update the manual.

And I think the first one is plain wrong (in the general case) :-) We don't know what the next version will be. Maybe they skip a version, add a new sub-minor version, or jump to the next major version?

+1 for a PR that updates the manual with this corner case: version number scheme for unreleased versions of a projects that do have releases.

~~@bjornfor: git describe isn't bullet-proof in general, unfortunately; for example: 7.1-363-gSomeHash < 7.1-72-gSomeHash.~~

EDIT: wrong. I'm sorry. Number clusters are detected by nix and compared numerically.

After upgrade `qapitrace` have working "Buffers" tab where the data can be inspected (it was always empty before). There is no tags after `7.1`, but I think that fixing pretty important piece of functionality warrants an upgrade to current `master` tip.

binarin · 2017-04-20T08:03:53Z

Updated version to git describe-produced string

FRidh · 2017-04-20T10:45:02Z

Why is there a need to use a different versioning scheme here? According to our guidelines the name part should be apitrace-unstable and the version part a date 2017-04-11.

This will break upgrading with nix-env from current versions to this version. That is, in my opinion, correct, because this is not a stable release. Therefore, in my opinion the correct solution here is to add an expression (or override) for this unstable version, next to the current stable version.

vcunat · 2017-04-20T11:30:07Z

I don't consider the nature of this change to imply creating a separate unstable package. We want nix-env -u to update to fix the bugs, and we want to be consistent when/if another release is made.

ocharles · 2017-04-26T10:10:48Z

I'm a little confused as to the state of this PR. Are we all onboard with the current naming?

vcunat · 2017-04-30T08:28:53Z

Right, I believe we converged on git describe being a good way.

ocharles self-requested a review April 11, 2017 15:33

ocharles approved these changes Apr 11, 2017

View reviewed changes

ocharles requested review from nckx and FRidh April 11, 2017 15:33

nckx approved these changes Apr 11, 2017

View reviewed changes

Mic92 reviewed Apr 13, 2017

View reviewed changes

apitrace: 7.1 -> git

3ca22aa

After upgrade `qapitrace` have working "Buffers" tab where the data can be inspected (it was always empty before). There is no tags after `7.1`, but I think that fixing pretty important piece of functionality warrants an upgrade to current `master` tip.

binarin force-pushed the apitrace-update branch from b2e21f8 to 3ca22aa Compare April 20, 2017 08:02

vcunat merged commit 4ae18e0 into NixOS:master Apr 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apitrace: 7.1 -> git #24829

apitrace: 7.1 -> git #24829

binarin commented Apr 11, 2017

mention-bot commented Apr 11, 2017

ocharles commented Apr 11, 2017

nckx left a comment

Mic92 Apr 13, 2017 •

edited

edolstra Apr 13, 2017

binarin Apr 14, 2017

Mic92 Apr 14, 2017 •

edited

vcunat Apr 14, 2017 •

edited

Mic92 Apr 14, 2017

bjornfor Apr 14, 2017

binarin Apr 14, 2017

bjornfor Apr 14, 2017

vcunat Apr 20, 2017 •

edited

binarin commented Apr 20, 2017

FRidh commented Apr 20, 2017

vcunat commented Apr 20, 2017 •

edited

ocharles commented Apr 26, 2017

vcunat commented Apr 30, 2017

apitrace: 7.1 -> git #24829

apitrace: 7.1 -> git #24829

Conversation

binarin commented Apr 11, 2017

Motivation for this change

Things done

mention-bot commented Apr 11, 2017

ocharles commented Apr 11, 2017

nckx left a comment

Choose a reason for hiding this comment

Mic92 Apr 13, 2017 • edited

Choose a reason for hiding this comment

edolstra Apr 13, 2017

Choose a reason for hiding this comment

binarin Apr 14, 2017

Choose a reason for hiding this comment

Mic92 Apr 14, 2017 • edited

Choose a reason for hiding this comment

vcunat Apr 14, 2017 • edited

Choose a reason for hiding this comment

Mic92 Apr 14, 2017

Choose a reason for hiding this comment

bjornfor Apr 14, 2017

Choose a reason for hiding this comment

binarin Apr 14, 2017

Choose a reason for hiding this comment

bjornfor Apr 14, 2017

Choose a reason for hiding this comment

vcunat Apr 20, 2017 • edited

Choose a reason for hiding this comment

binarin commented Apr 20, 2017

FRidh commented Apr 20, 2017

vcunat commented Apr 20, 2017 • edited

ocharles commented Apr 26, 2017

vcunat commented Apr 30, 2017

Mic92 Apr 13, 2017 •

edited

Mic92 Apr 14, 2017 •

edited

vcunat Apr 14, 2017 •

edited

vcunat Apr 20, 2017 •

edited

vcunat commented Apr 20, 2017 •

edited