{buildPostgresql,orioledb}: init#513940
Conversation
wolfgangwalther
force-pushed
the
postgresql-overrides
branch
from
086ef8e to
4e71fb4
Compare
nixpkgs-ci
Bot
added
8.has: package (new)
wolfgangwalther
force-pushed
the
postgresql-overrides
branch
from
4e71fb4 to
f6b58bb
Compare
|
|
|
| let | ||
| attrName = if jitSupport then "${version}_jit" else version; | ||
| postgresql = buildPostgresql (import path); | ||
| postgresql = buildPostgresql (import path { inherit (self) fetchFromGitHub lib; }); |
There was a problem hiding this comment.
Is there any good reason left that I'm forgetting / not seeing on why we have this weird self construct here?
Doesn't have to be done in this PR, but I wonder if we can replace this with a normal callPackage interface by now.
There was a problem hiding this comment.
I briefly looked into that when writing the PR and it seemed to me that we need self when creating the new scope for the package set. I did not look at it in-depth, though.
| }; | ||
|
|
||
| buildPostgresql = | ||
| args: self.callPackage (import ./generic.nix args) { inherit buildPostgresql self; }; |
There was a problem hiding this comment.
Why are not implementing the finalAttrs interface here?
There was a problem hiding this comment.
I assume you mean why I'm not using lib.extendMkDerivation on buildPostgresql?
I thought a lot about this and I don't see a way to make it work:
- the builder needs to reference
this, which is a reference to same derivation, to make thewithJIT,withoutJITandwithPackagesmodifiers work. - this can't use
finalAttrs.finalPackage, because that one does not provide.override- it's just a derivation, but nothing wrapped in acallPackagecall - we tried and had to revert that fix, - to provide
.overrideit will need to ultimatelycallPackageon the builder expression again - and at this stage anyoverrideAttrswill be lost.
This means I need a different way to pass these arguments along to the builder, which is what I'm doing here: These args are wrapped around the callPackage interface (kind of the inverse of where finalAttrs would be, related to callPackage).
I tried a lot of different things, but was not able to make any work. I might be missing something obvious, though...
Moves the function to a separate file and makes it callPackage-able. Now that buildEnv also supports finalAttrs, the finalPackage construct can also be rewritten that way. This gives us clear separation of concern between the package builder in generic.nix and wrapper.nix.
Removes all the indentation in generic.nix, which is possible by changing the `self.callPackage` call to point at the file itself, not the `generic` function.
Exposes the postgresql builder function as a top-level argument. Not really useful that much, so far, because it needs some more arguments to allow passing relevant configuration. Split into multiple commits for ease of review. Eventually, this will allow to build forks of PostgreSQL including the whole package set machinery. This is not possible via overrideAttrs / override, because of the withPackages wrapper, which does support either of the two properly - and can't be made to do that, at least not by me.
Exposes the minimal set of arguments required to effective build a forked PostgreSQL. Considerations were: - Putting the `fetchFromGitHub` call as a default value for the `src` argument would have made all the arguments hash, rev and src optional, which is an odd interface. A little duplication in each of the versioned files is tolerable, imho. - Putting the `meta` information in a default value would possibly lead to people forgetting to set it when packaging forks, which would lead to wrong metadata. It would also lead to some forks be attributed to the postgres team, but not others. We should rather be explicit about whether the postgres team wants to maintain all forks or none of them. I went with "none of them" here and moved the setting of meta.teams to the versioned files instead. - My best guess is that forks of PostgreSQL must be published under the same license, so I kept that in the builder. It's overridable, though.
We have dropped everything below v14 by now. To not cause rebuilds, I left the order in the list alone.
wolfgangwalther
force-pushed
the
postgresql-overrides
branch
from
f6b58bb to
38ee3fc
Compare
nixpkgs-ci
Bot
added
the
2.status: merge conflict
|
@wolfgangwalther hey, do you plan on picking this PR back up? |
Not exactly sure what you'd expect me to do at this stage. From my perspective this PR requires review, which will take "as long as it takes". |
3 participants
I want to package
orioledb, which is an extension that needs a patched version of postgresql. In other words, I need to package a postgresql fork.The first thing I tried downstream was something like
(postgresql.overrideAttrs { src = ... orioledb/postgres ... }).withPackages (pkgs: pkgs.callPackage (... orioledb/orioledb ...)). Unfortunately that doesn't work, becausewithPackagesloosesoverrideAttrs. We tried to fix that in #488692, but failed - the interactions betweenwithPackagesand overriding are more complicated.I tried a few different things to fix that, but ultimately failed with all of them. So I went this way: We're now exposing a separate
buildPostgresqlfunction, which can be used to call the generic builder. Also addedorioledbas an example of its usage.(more text in the commits!)
Things done
TODO: Investigate aarch64-related failure for orioledb.
nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.