fetchFromGit{Hub,Lab}: add meta.changelog by Synthetica9 · Pull Request #144141 · NixOS/nixpkgs

Synthetica9 · 2021-11-01T16:45:54Z

Motivation for this change

Things done

Script used: ag -l --nix \ | xargs grep --files-with-match "inherit (src.meta) homepage" \ | xargs grep --files-with-match --perl-regexp 'version = "[0-9\.]+"' \ | xargs grep --files-with-match --perl-regexp "fetchFromGit(Hub|Lab)" \ | xargs grep --files-without-match changelog \ | xargs sed -i "s/inherit (src.meta) homepage;/inherit (src.meta) homepage changelog;

rnhmjoj · 2021-11-03T14:18:19Z

Please, consider documenting the meta output in the "fetches" chapter. I had no idea this was even a thing.

mweinelt · 2021-11-03T14:20:26Z

I usually rely on the plain URL being around in meta to quickly navigate to the project home or its changelog when editing or reviewing a package. While inheriting these values from the fetchers is a neat trick it does not improve the developer experience in my opinion.

06kellyjac · 2021-11-03T14:22:01Z

I usually rely on the plain URL being around in meta to quickly navigate to the project home or its changelog when editing or reviewing a package. While inheriting these values from the fetchers is a neat trick it does not improve the developer experience in my opinion.

I second this. I used to like the quick and convenience of inherit (src) homepage; as a package-er but as a reviewer it is very nice to just click (or whatever) the link rather than go to search.nixos.org find the package and click the homepage there, or type out the address by hand based on the fetchFromGithub values

roberth · 2021-11-03T14:24:37Z

It's ok for the fetcher to add this to its meta, but packages should provide their own changelog, because often enough, the github releases page is not the best changelog. For instance nix should have https://nixos.org/manual/nix/stable/release-notes/release-notes.html (which is an empty "header" page that should be fixed but that's beside the point)

SuperSandro2000 · 2021-11-03T14:33:04Z

Please, consider documenting the meta output in the "fetches" chapter. I had no idea this was even a thing.

That would encourage people to add more occurrences of this which I am against. As hexa- already wrote using inherit is pretty bad for the review experience and copying URLs together is just annoying. If I have a direct link to the source I usually do more research myself to give better review comments. If there is no such link I put the extra work on the contributor.

Synthetica9 · 2021-11-03T14:33:45Z

It's ok for the fetcher to add this to its meta, but packages should provide their own changelog, because often enough, the github releases page is not the best changelog.

This adds a changelog to packages that previously didn't have one, so while I guess that some packages do indeed have a better changelog somewhere, this isn't listed in meta.

I usually rely on the plain URL being around in meta to quickly navigate to the project home or its changelog when editing or reviewing a package. While inheriting these values from the fetchers is a neat trick it does not improve the developer experience in my opinion.

Changelog are usually version-specific, and thus not clickable even when they do specify a "full" url in the derivation due to interpolation with ${version}

$ grep  -r  'changelog = "https://' pkgs  | wc -l
596
$ grep  -r  'changelog = "https://' pkgs  | grep '${version}' | wc -l
448

75%+ of "changelog =" entries are not clickable currently.

I agree that it would be nice to have a cleaner way to open (derived) urls in a package.

mweinelt · 2021-11-03T14:42:16Z

Fair enough regarding the changelog. I agree this solution is elegant when changelogs are provided through GitHub releases. I just think we need to roll this back for the homepage, and maybe clarify homepage vs source repository.

06kellyjac · 2021-11-03T14:44:38Z

75%+ of "changelog =" entries are not clickable currently.

it's way easier to click and then fix the link or just go to releases than inherit (src) changelog;

roberth · 2021-11-03T14:46:42Z

Changelog URLs computed from a version are also a problem because a PR may need multiple such pages to be reviewed and it's easy to forget about those other pages.

pkgs/applications/audio/keyfinder-cli/default.nix

mweinelt · 2021-11-03T14:47:50Z

pkgs/applications/misc/clipit/default.nix

  meta = with lib; {
    description = "Lightweight GTK Clipboard Manager";
-    inherit (src.meta) homepage;
+    inherit (src.meta) homepage changelog;


https://github.com/CristianHenzel/ClipIt/releases/tag/45e2ea386d04dbfc411ea370299502450d589d0c

HTTP 404.

we should just be fetching from the tag here

mweinelt · 2021-11-03T14:51:35Z

pkgs/applications/misc/subsurface/default.nix


    meta = with lib; {
-      inherit (src.meta) homepage;
+      inherit (src.meta) homepage changelog;


https://github.com/vladest/googlemaps/releases/tag/8f7def10c203fd3faa5ef96c5010a7294dca0759

HTTP 404.

mweinelt · 2021-11-03T14:56:44Z

pkgs/applications/video/epgstation/default.nix

    passthru.updateScript = import ./update.nix {
      inherit lib;
-      inherit (src.meta) homepage;
+      inherit (src.meta) homepage changelog;


Not sure the update script needs the changelog.

mweinelt · 2021-11-03T14:57:06Z

pkgs/applications/video/mirakurun/default.nix

  passthru.updateScript = import ./update.nix {
    inherit lib;
-    inherit (src.meta) homepage;
+    inherit (src.meta) homepage changelog;


Not sure this update script needs the changelog either.

Ah no, it doesn't 😅

mweinelt · 2021-11-03T15:02:20Z

pkgs/development/libraries/libglvnd/default.nix

      Both GLX and EGL are supported, in any combination with OpenGL and OpenGL ES.
    '';
-    inherit (src.meta) homepage;
+    inherit (src.meta) homepage changelog;


https://gitlab.freedesktop.org/glvnd/libglvnd/-/releases/v1.3.4/

HTTP 404.

mweinelt · 2021-11-03T15:05:18Z

pkgs/os-specific/linux/fscryptctl/legacy.nix

      documentation for filesystem encryption before using fscryptctl.
    '';
-    inherit (src.meta) homepage;
+    inherit (src.meta) homepage changelog;


"https://github.com/google/fscryptctl/releases/tag/v{version}"

Broken URL.

Pebcak? https://github.com/google/fscryptctl/releases/tag/v0.1.0

nix-repl> pkgs.fscryptctl.meta.changelog "https://github.com/google/fscryptctl/releases/tag/v{version}"

wait no, wtf is happening here

Also this package should be dropped accoding to the comment?

Oops, right, that package (fscryptctl-experimental) should be dropped (I forgot to include my GH handle in the TODO which I use to find them) and it seems like I forgot the $ in the fscryptctl package. I've opened #144885 if that helps. You can also include it in this PR if you want.

mweinelt · 2021-11-03T15:08:01Z

pkgs/tools/filesystems/vmfs-tools/default.nix

    license = licenses.gpl2;
    platforms = platforms.linux;
-    inherit (src.meta) homepage;
+    inherit (src.meta) homepage changelog;


https://github.com/glandium/vmfs-tools/releases/tag/4ab76ef5b074bdf06e4b518ff6d50439de05ae7f

HTTP 404

Ah the version here should be changed to -unstable

06kellyjac · 2021-11-03T15:52:23Z

#144141 (comment)

Yeah, we can't rely on all tags having releases

https://gitlab.freedesktop.org/glvnd/libglvnd/-/releases/v1.3.2 is the latest tag with a release

Also I guess adding this as default to all sources means even if we manually set the changelog in the package meta, maybe to a CHANGELOG.md or w/e the src's changelog value will be completely wrong

Maybe it should be called release or releasePage? but then it'd be changelog = src.meta.release which I guess isn't too bad

github-actions bot added the 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) label Nov 1, 2021

Synthetica9 changed the title ~~fetchFromGitHub: add meta.changelog~~ fetchFromGit{Hub,Lab}: add meta.changelog Nov 1, 2021

Synthetica9 mentioned this pull request Nov 1, 2021

wlroots: move to new home on freedesktop #144142

Closed

12 tasks

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 1, 2021

Synthetica9 requested review from FRidh, jonringer, marsam, thoughtpolice and ttuegel as code owners November 1, 2021 17:29

Synthetica9 force-pushed the fetchfromgithub-changelog branch from ff9da16 to 58bd1e4 Compare November 1, 2021 17:31

Synthetica9 added 3 commits November 1, 2021 18:32

fetchFromGitHub: add meta.changelog

1d28701

fetchFromGitLab: add meta.changelog

2dbc010

Synthetica9 force-pushed the fetchfromgithub-changelog branch from 58bd1e4 to b5bc1fa Compare November 1, 2021 17:32

mweinelt requested changes Nov 3, 2021

View reviewed changes

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 19, 2022

wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

wegank marked this pull request as draft March 21, 2024 14:01

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024

Uh oh!

Conversation

Synthetica9 commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

rnhmjoj commented Nov 3, 2021

Uh oh!

mweinelt commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

06kellyjac commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roberth commented Nov 3, 2021

Uh oh!

SuperSandro2000 commented Nov 3, 2021

Uh oh!

Synthetica9 commented Nov 3, 2021

Uh oh!

mweinelt commented Nov 3, 2021

Uh oh!

06kellyjac commented Nov 3, 2021

Uh oh!

roberth commented Nov 3, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Synthetica9 Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

06kellyjac commented Nov 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Synthetica9 commented Nov 1, 2021 •

edited

Loading

mweinelt commented Nov 3, 2021 •

edited

Loading

06kellyjac commented Nov 3, 2021 •

edited

Loading

Synthetica9 Nov 3, 2021 •

edited

Loading