The new packages LGTM, but I don't know about maven packaging. And do we have to unconditionally link them into postgresql_jdbc and keycloak, or can users add these manually to nixos configurations such as services.keycloak.plugins?

The new packages LGTM, but I don't know about maven packaging. And do we have to unconditionally link them into postgresql_jdbc and keycloak, or can users add these manually to nixos configurations such as services.keycloak.plugins?

The rationale behind linking them to postgresql_jdbc is that this way, every Nix package that uses postgresql_jdbc gains automatically the capability of supporting PostgreSQL connection via Unix sockets. I discussed this in the Matrix channel (with @Infinidoge) where this idea was considered to be a good idea, as in Nix we generally make the default full-featured, if the additional requirements are small enough (and junixsocket is pretty small).

In keycloak, we have the problem that it does not use the postgresql_jdbc Nix package, but instead ships its own postgresql_jdbc version - and that it is not easy (or even feasible) to replace it.

But apart from that detail, the same rationale is applied here: Yes, we could leave junixsocket out, so everyone would have to add it via the plugins themselves. But I can't think of any sane setup (given that keycloak is a security-relevant application!) where you would run keycloak's database on a different machine than keycloak itself, and hence where you would want to connect it to its database via TCP/IP when you could connect it directly via Unix sockets.

But, both integrations into postgresql_jdbc and keycloak are just proposals, and hence separate commits. If those are perceived as "too controversial" rather than "sane defaults", we could of course leave them out.

@Infinidoge @NickCao Do you have any decision on that topic? Should I reduce my PR, or should we keep the out-of-the-box unix socket support for all postgresql-jdbc consumers and keycloak.

@Infinidoge @NickCao Do you have any decision on that topic? Should I reduce my PR, or should we keep the out-of-the-box unix socket support for all postgresql-jdbc consumers and keycloak.

I'd say let's leave the out-of-the-box unix socket support for later.

I'd say let's leave the out-of-the-box unix socket support for later.

@NickCao Done. Please review again.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 4e8b7b9afbef7b9f9b18366cef2bce6493d988a4

aarch64-darwin

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 4e8b7b9afbef7b9f9b18366cef2bce6493d988a4

aarch64-linux

One more nitpick: for some reason the meta attributes are not passed to the final derivation:

nix-repl> :p junixsocket-common.meta
{
  available = true;
  broken = false;
  insecure = false;
  maintainers = [ ];
  name = "com_kohlschutter_junixsocket_junixsocket-common-2.10.1";
  outputsToInstall = [ "out" ];
  position = "/home/nickcao/Projects/nixpkgs/pkgs/build-support/fetchmavenartifact/default.nix:82";
  unfree = false;
  unsupported = false;
}

for some reason the meta attributes are not passed to the final derivation

Is this because fetchMavenArtifact is top-level and not wrapped in stdenv.mkDerivation?

for some reason the meta attributes are not passed to the final derivation

Is this because fetchMavenArtifact is top-level and not wrapped in stdenv.mkDerivation?

Yeah, but I expected fetchMavenArtifact to do it.

I made another pr (PR #433781) based on this PR because I was inspired by the brevity of top-level fetchMavenArtifact, but I rewrote it to use src = fetchMavenArtifact { ... }insidestdenv.mkDerivation`.

@vog: you might want to try the same modification, see: https://github.com/NixOS/nixpkgs/compare/cb87cf22932de66cdb347958037d05f0c5bdafe6..2e37e33388ef62b367448285bb97fd559e9709cd

I made another pr (PR #433781) based on this PR because I was inspired by the brevity of top-level fetchMavenArtifact, but I rewrote it to use src = fetchMavenArtifact { ... }insidestdenv.mkDerivation`.

@vog: you might want to try the same modification, see: https://github.com/NixOS/nixpkgs/compare/cb87cf22932de66cdb347958037d05f0c5bdafe6..2e37e33388ef62b367448285bb97fd559e9709cd

I'm not sure if we should really go that route.

Wouldn't it make more sense to instead fix fetchMavenArtifact to pass the metadata?

I made another pr (PR #433781) based on this PR because I was inspired by the brevity of top-level fetchMavenArtifact, but I rewrote it to use src = fetchMavenArtifact { ... }insidestdenv.mkDerivation`.
@vog: you might want to try the same modification, see: https://github.com/NixOS/nixpkgs/compare/cb87cf22932de66cdb347958037d05f0c5bdafe6..2e37e33388ef62b367448285bb97fd559e9709cd

I'm not sure if we should really go that route.

What are your reservations?

Wouldn't it make more sense to instead fix fetchMavenArtifact to pass the metadata?

I don't really know which is the best way. But I don't see any packages in nixpkgs that are doing it that way, nor can I find any documentation that definitively says what is the best thing to do in this case.

If you think you can make it work and get approved by people with more experience and authority than me, go for it. I'm curious what that would look like, so if you think it can be done I'd say it's worth doing just as an experiment.

I made another pr (PR #433781) based on this PR because I was inspired by the brevity of top-level fetchMavenArtifact, but I rewrote it to use src = fetchMavenArtifact { ... }insidestdenv.mkDerivation`.
@vog: you might want to try the same modification, see: https://github.com/NixOS/nixpkgs/compare/cb87cf22932de66cdb347958037d05f0c5bdafe6..2e37e33388ef62b367448285bb97fd559e9709cd

I'm not sure if we should really go that route.

What are your reservations?

This simply doesn't make any sense to me. It is cumbersome and wasteful.

fetchMavenArtifact already creates a derivation that contains exactly the files we want, in the structure we want, with the naming we want. Just the metadata is missing.

But instead of improving that function to just pass down the missing metadata, more code is written, to create another derivation, into which all binary files are copied, so now they are twice in the store, at least until the next "gc" or "optimize" action. And all that extra machinery just because the original derivation is missing an attribute.

To me, that current way requires justification. Why the hell people are doing that? It is cumbersome and wasteful. Do I overlook something?

Wouldn't it make more sense to instead fix fetchMavenArtifact to pass the metadata?

I don't really know which is the best way. But I don't see any packages in nixpkgs that are doing it that way, nor can I find any documentation that definitively says what is the best thing to do in this case.

Yes, that's what I found confusing, too. But the missing metadata explains this at least to some level.

If you think you can make it work and get approved by people with more experience and authority than me, go for it. I'm curious what that would look like, so if you think it can be done I'd say it's worth doing just as an experiment.

Indeed, I'll go ahead and try to fix that in another PR, then let's see if this is acceptable, or if anyone raises valid concerns that I may have overlooked.

Indeed, I'll go ahead and try to fix that in another PR, then let's see if this is acceptable, or if anyone raises valid concerns that I may have overlooked.

@NickCao @msgilligan

Done! See PR #433975.

To me, [the] current way requires justification. Why the hell people are doing that? It is cumbersome and wasteful. Do I overlook something?

Yes, that's what I found confusing, too.

I'm just looking to learn the current state-of-the-Nix-art, you're looking to push it forward. Awesome!

(Eventually, of course, I would like to see more of these Maven artifacts being built from source, but having a simple mechanism to bring in binaries is nice, too -- and an important stepping stone.)

I'll take a look at PR #433975.

I'm just looking to learn the current state-of-the-Nix-art, you're looking to push it forward. Awesome!

(Eventually, of course, I would like to see more of these Maven artifacts being built from source, but having a simple mechanism to bring in binaries is nice, too -- and an important stepping stone.)

I'll take a look at PR #433975.

Ok, it's done!

PR #433975 has been accepted, a I rebased this one onto the latest master.

@msgilligan @NickCao Is there anything else to be done for this PR?

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 1838087f6899480b2d52a6e9c819a3cdb5d96ea8

aarch64-darwin

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 1838087f6899480b2d52a6e9c819a3cdb5d96ea8

aarch64-linux

A minor request: add a unique description to junixsocket-native-common.

@msgilligan Ok, done! Please review again.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 72427e253359bfcd4474fbcb0b5d725c5d61555c

aarch64-darwin

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 427538
Commit: 72427e253359bfcd4474fbcb0b5d725c5d61555c

aarch64-linux

Successfully created backport PR for release-25.05:

• [Backport release-25.05] junixsocket{,-native}-common: init at 2.10.1 #438867