Nixpkgs: Package upgrades: Request link to changelog in commit message?

Created on 17 Nov 2018  Â·  14Comments  Â·  Source: NixOS/nixpkgs

Note: The discussion appears to be at a state of everyone agreeing that putting a Changelog tag in the commit is a bad idea. See eg. https://github.com/NixOS/nixpkgs/issues/50483#issuecomment-439603594 for an alternative way to put the same information at a correct place.


Issue description

It is currently quite hard to know what exactly changes during package bumps when reviewing the commits, be it during a PR review or after-the-fact, when re-reading the commits, as there is often no link to a Changelog file easily accessible.

If the Changelog was necessarily present, it'd mean that reviewers could easily go and read it to check whether an update needs backporting (eg. whether it fixes security issues). It'd also make it much easier for people who follow the git history of nixpkgs to figure out what changed in a package update, without having to scout the web for a Changelog.

Proposed solution

Request that every package bump include, in its commit message, Changelog: [url of the changelog], or Changelog: none if upstream doesn't provide a Changelog.

This is a policy change, and will likely require quite some time to get all reviewers to check for the presence of the Changelog tag.

Future work

@GrahamcOfBorg can be useful here by adding a check for commits that have -> in their title, checking for a Changelog tag. However, it should not display as a failed check if it's absent, as the “-> in the title” heuristic may catch non-upgrade commits.

It may also be possible to, later on, request that people who submit PRs add in a Security fix: [CVE or URL list] or Security fix: none, to make it easier for specifically security fixes. I didn't propose this straight away because it requires potentially more work to figure out, and sometimes the security fixes aren't even known at the time the update is made.

Finally, this is part of a long-term plan of mine to revive the “Security fixes from 20xx-xx-xx” series, by making it easier to figure out which commits introduce security fixes.

Alternatives

It would also be possible to request a link to the changelog in a meta.changelog field, but changelogs may change location between versions, so it wouldn't be a good place to put it in if we want automated tools to check that the Changelog link is always up-to-date.

On the other hand, requesting it to just always be present in the commit message, even if it is the same Changelog as the previous one, means that there must always be a conscious effort of searching for it.

Drawbacks

It's more burden when bumping a package, as it also requires fetching the Changelog. This could be mitigated by actually adding a meta.changelog item that would hold the link to the Changelog, for packages for which it is known not to change often.

What do you think about it?

stale community feedback

Most helpful comment

If the main purpose is to facilitate the review process wouldn't it be sufficient to request a changelog link or comment in the PR message? Introducing an extra meta tag seems like a heavy solution. Links can go stale after while (nixpkgs carries a lot of dead links), which would render them useless and just increase the maintenance workload.

All 14 comments

It would also be possible to request a link to the changelog in a meta.changelog field, but changelogs may change location between versions, so it wouldn't be a good place to put it in if we want automated tools to check that the Changelog link is always up-to-date.

On the other hand, requesting it to just always be present in the commit message, even if it is the same Changelog as the previous one, means that there must always be a conscious effort of searching for it.

Drawbacks

It's more burden when bumping a package, as it also requires fetching the Changelog. This could be mitigated by actually adding a meta.changelog item that would hold the link to the Changelog, for packages for which it is known not to change often.

  1. I think allowing meta entries about changelog should happen before policy changes requiring changelog, in the sense that things that can be done cheaply and only once should be done before falling back to a more costly solution.

  2. I think there might be an intermediate case: there is a stable page that doesn't change often, that contains changelog. I mean a layout like https://kernel.org/ — maybe it would also be good to reflect in meta. Changelog never existing might also be a good thing to record, to avoid making a thorough check every time instead of every year.

  3. In the ideal world >50% of updates will be done by r-ryantm or equivalents. How the case when the bot does the update and testing — and changelog is always in a different place — should be handled?

  1. I think we agree here, but kind of preferred to ask for general opinions about a changelog before sending in a PR, especially as I'm not sure the fallback of my last PR changing meta is over yet :)
  2. For kernel.org, I'd think one could just point the meta.changelog (or Changelog: in the commit, depending on what the outcome of this discussion gives) url to this page: it's meant to be used by a human anyway, using it automatically would require much more metadata I'm not sure we'd be willing to input.
  3. In this case, I think the bot should be taught how to find the changelog. If it can't, anyway a human will have to go and seek the changelog to figure out whether the change needs to be backported, so that human could also commit --amend to add the changelog while they're at it…?

Also, thinking about it, maybe for user-friendliness it may make sense to actually have all changelog entries be in meta.changelog, with a format like:

{
  # ...
  meta.changelog = {
    "1.2.3 -> 1.2.4" = https://example.org/changelog;
    "1.2.4 -> 2.3.5" = https://example.org/changelog-v2;
  };
}

And then either add an entry to the “list” or increase the version number of the last entry upon an update.

This would make display of changelog lists possible on eg. the packages page.

Maybe this would be better than using the commit messages for this information?

And having = null for version upgrades that have no changelog, so that automated tooling could warn if the meta.changelog set is the same before and after the upgrade.

If the main purpose is to facilitate the review process wouldn't it be sufficient to request a changelog link or comment in the PR message? Introducing an extra meta tag seems like a heavy solution. Links can go stale after while (nixpkgs carries a lot of dead links), which would render them useless and just increase the maintenance workload.

I'm in favor of adding a changelog or releaseNotes meta entry. E.g. this for postgresql:

{
  meta.changelog = "https://www.postgresql.org/docs/current/release.html";
}

Which will also allow people to more easily link the changelog in the PR.

@Ekleog I think structured meta entries are not supported?

Amending commits is not a straightforward approach, as the person to review would often be a person without access to updater bot branches.

I would say that direct link to changelog vs indirect link to a page that usually has the changelog might be a useful distinction for expectation management. Or maybe not.

There is a difference between the people writing packages and those consuming packages.

For consumers, everyone can see that it's great to have such links. Especially as a specific URL that doesn't change or perhaps even a copied changelog (because URLs can die).

For producers, it's just more work (and it's not exactly a creative activity). It's something that just needs to be automated or not done at all, IMHO.

As such, I am very much against this. The idea to increase the amount of work when there is still a pile of work does not compute for me. No, the way to run an organisation or process is to increase efficiency first (for example by doing what ryantm is doing which sort of seems to be getting more broader acceptance) and only then to raise either the volume of products (= more packages) or the quality (=adding a changelog).

Increasing work load without the required efficiency improvements in the process does not scale. All it will do is create a greater risk for maintainers to leave and for NixOS to rot away.

In general, I think every such suggestion increasing the amount of work to be done without obvious improvements in efficiencies that happened before (for example more committers might also count as a process improvement) should be closed without discussion. A lot of similar ideas have come by and they have all failed.

If we want something useful to come out of this issue, I think we should have the policy to as quickly as possible close such "ideas".

@7c6f434c My reading (coming from https://github.com/NixOS/nix/issues/2532) of https://github.com/NixOS/nix/blob/6924bdf2bf8f50f2e6ec5d490571594450aba13a/src/libexpr/get-drvs.cc#L166 is that both lists (eg. maintainers) and attrsets (which I don't think we have right at top-level now indeed, but we do have these as eg. items of the maintainers list) are allowed, so I don't think it'd be impossible per se :)

@coretemp The package updater must already look for the changelog anyway. If they don't, they aren't doing their job properly, because an update might include security fixes and need to be backported. Requiring that the changelog is explicitly updated helps ensure updaters actually do their job and don't make the things worse for everyone by merging a security update in master only, making people assume they checked the changelog.

@coretemp The package updater must already look for the changelog anyway. If they don't, they aren't doing their job properly, because an update might include security fixes and need to be backported. Requiring that the changelog is explicitly updated helps ensure updaters actually do their job and don't make the things worse for everyone by merging a security update in master only, making people assume they checked the changelog.

I wonder what is the share of contributors who never use stable, never going to use stable, and are never sure if stable needs a backport in the corner cases… I definitely don't actually care about stable even if I try to remember in the clear-cut cases.

Anyway, sooner or later some user will wonder whether a backport is needed; so having a way to store the changelog habits of a project is a net process improvement. Once we see how it works out, then maybe we could automate nagging about the changelogs for updates.

Maybe the changelog nagging will sometimes be investigated by people who want to contribute but lack confidence to write code or docs…

I wonder what is the share of contributors who never use stable, never
going to use stable, and are never sure if stable needs a backport in
the corner cases… I definitely don't actually care about stable even
if I try to remember in the clear-cut cases.

That's actually part of the reason I want changelogs easily accessible:
to be able to easily review the changelogs of (at least part of) the
updates that are merged into master to scan through for security fixes,
so that even if one backport is forgotten someone would hopefully
notice. (not saying “I” because I know I won't reasonably have time to
review more than a handful of commits compared to the mass merged, but
if enough people do the same we should cover a statistically significant
portion of the bumps)

Are there actually that many packages that put their changelogs "at a different place every version"? If we accept links to summary pages such as kernel.org, I imagine 99% of cases wouldn't need manual intervention and could be handed by r-ryantm.

Can this issue get closed now since https://github.com/NixOS/nixpkgs/pull/60371 is merged and an issue in https://github.com/NixOS/ofborg/issues/416 exists?

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

If there is anything left to be done, that should live in the RFC repo.

Was this page helpful?
0 / 5 - 0 ratings