Skip to content

pkgs/README: Document commit prefixes & automatic CI builds#431688

Merged
wolfgangwalther merged 1 commit intoNixOS:masterfrom
doronbehar:doc/git-cmt-prefixes
Aug 9, 2025
Merged

pkgs/README: Document commit prefixes & automatic CI builds#431688
wolfgangwalther merged 1 commit intoNixOS:masterfrom
doronbehar:doc/git-cmt-prefixes

Conversation

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Aug 7, 2025

I found myself explaining these details very often through out my time with Nixpkgs, and it always felt a bit weird for me that these are document in a different repository. So I decided to slightly copy that content to Nixpkgs, with a refresh set of examples.

I'm asking for a review from a few people who were involved in #400934, as this PR should be tangent but still may be slightly related:

@nix-owners nix-owners bot requested a review from infinisil August 7, 2025 08:31
@nixpkgs-ci nixpkgs-ci 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. 6.topic: policy discussion Discuss policies to work in and around Nixpkgs labels Aug 7, 2025
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These guidelines are specific to pkgs/ - they don't apply to all the other parts of nixpkgs. Thus, I think the right place to put them is in the specific conventions for this subfolder at https://github.com/NixOS/nixpkgs/tree/master/pkgs#commit-conventions.

@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch from 4245d3c to d74f425 Compare August 7, 2025 09:51
@doronbehar doronbehar changed the title CONTRIBUTING: Document Git commit prefixes & Automatic CI Builds pkgs/README: Document commit prefixes & automatic CI builds Aug 7, 2025
pkgs/README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the following:

nix-repl> python3Packages.recurseForDerivations
false

nix-repl> python312Packages.recurseForDerivations
true

nix-repl> python313Packages.recurseForDerivations
true

So this explanation doesn't quite line up, yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this is indeed confusing. The ofborg README also doesn't give as examples python3Packages, but here's an example were ofborg created (as always) a PR with python3Packages in the prefix and ofborg did (at least attempted) to build the packages:

See also:

Let's ask @Artturin for help specifically on this.

Copy link
Member

@pbsds pbsds Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aliases like python3Packages.foobar didn't use to work before the ofborg migration necessitating e.g. python313Packages.foobar, but now it does. More context in NixOS/ofborg#577. Since that issue was closed r-ryantm was switched to the python3Packages alias which avoids a lot of duplicate PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pbsds for commenting. I sort of figured that out, but it's nice to link that issue too. The questions making this review conversation still open, are: What is the parameter that dictates if ofborg will build an attribute or not? Is it whether they are recursed to or not and python3Packages is an exception hard-coded in ofborg's code? Or is it due to something more subtle the way we define these attribute sets in all-packages.nix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think by now ofborg just builds what it gets. You tell it to build a.b.c: init and it will build a.b.c, no questions asked. That's because it doesn't do any eval step anymore, that could do any of the recursion parts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be true nowadays indeed. Let's wait and see:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed ofborg has no difference between python3.pkgs and python3Packages and the rest! I tested also a gotify-server.ui: commit, where gotify-server.ui is completely not recursed into by Hydra, and it built it too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The python docs seem to give outdated advice on this python3Packages thing:

* The current default version of python should be included
in commit messages to enable automatic builds by ofborg.
For example `python313Packages.numpy: 1.11 -> 1.12` should be used rather
than `python3Packages.numpy: 1.11 -> 1.12`.
Note that `pythonPackages` is an alias for `python27Packages`.

I guess we should update these, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch from d74f425 to 39c2a4a Compare August 7, 2025 10:28
@wolfgangwalther wolfgangwalther dismissed their stale review August 7, 2025 10:40

hard blockers had been addressed, although some questions still remain.

@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch from 39c2a4a to 626b9a6 Compare August 7, 2025 13:58
@doronbehar
Copy link
Contributor Author

Thanks for your reviews, the whole recursedInto subject indeed is no longer relevant and the paragraph before the table with the example is much more simple now.

@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch 3 times, most recently from 2f65f9e to 34fd7b9 Compare August 7, 2025 14:51
@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch from 34fd7b9 to c41ee2b Compare August 7, 2025 14:57
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 7, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 7, 2025
Somewhat based upon ofborg's README contents:

https://github.com/NixOS/ofborg?tab=readme-ov-file#automatic-building

Co-authored-by: Wolfgang Walther <walther@technowledgy.de>
Co-authored-by: Peder Bergebakken Sundt <pbsds@hotmail.com>
@doronbehar doronbehar force-pushed the doc/git-cmt-prefixes branch from c41ee2b to 6857dc8 Compare August 7, 2025 16:28
@wolfgangwalther
Copy link
Contributor

Since this describes what is already fact and is "just" an updated documentation of what is available in the ofborg repo, I consider this uncontroversial enough in principle to not wait for more community feedback.

@wolfgangwalther wolfgangwalther merged commit 4fc07c2 into NixOS:master Aug 9, 2025
26 of 28 checks passed
drupol

This comment was marked as resolved.

@drupol
Copy link
Contributor

drupol commented Aug 9, 2025

Please ignore my previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 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. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants