protege: init at 5.6.4 #316647
protege: init at 5.6.4 #316647
Conversation
ofborg
bot
added
8.has: package (new)
pkgs/by-name/pr/protege/package.nix
Outdated
| stdenvNoCC.mkDerivation rec { | ||
| inherit pname version; | ||
|
|
||
| buildInputs = [ protege-build ]; |
There was a problem hiding this comment.
This is a bit weird, perhaps it'd be possible to combine the installPhase of this derivation to the installPhase of this one?
There was a problem hiding this comment.
It is, but I wanted to keep them separate according to the following reasoning:
- the result of the Maven build contains a target-agnostic directory containing all the necessary jars plus a launcher script for every platform
- what distinguishes an installation for a certain platform from one for another platform is just how the produced files are placed in the filesystem (e.g. where the icon files are copied)
So why not produce a perfectly agnostic build that is the same for all platforms, and then produce a platform-specific symlink structure that refers to its relevant components as part of a platform-specific derivation? This way, when Darwin systems will be supported by this package, both Linux and Darwin builds would share the same core derivation and access it via symlinks, organized according to their platform-specific filesystem conventions. This can save space and, potentially, computation, as a single multi-target derivation would need to be evaluated for every end platform and would duplicate perfectly agnostic Java bytecode for no reason.
I don't know if my explanation is clear...
There was a problem hiding this comment.
It is, but I wanted to keep them separate according to the following reasoning:
* the result of the Maven build contains a target-agnostic directory containing all the necessary jars plus a launcher script for every platform
How are you so sure that this protege-build is the same for all platforms?
* what distinguishes an installation for a certain platform from one for another platform is just how the produced files are placed in the filesystem (e.g. where the icon files are copied)
I'm not sure you gain anything with this. Do you want to save a few kilobytes on Hydra? What's important, is that both platforms have the same mvnHash.
So why not produce a perfectly agnostic build that is the same for all platforms, and then produce a platform-specific symlink structure that refers to its relevant components as part of a platform-specific derivation? This way, when Darwin systems will be supported by this package, both Linux and Darwin builds would share the same core derivation and access it via symlinks, organized according to their platform-specific filesystem conventions. This can save space and, potentially, computation,
Yes I agree with the computation argument, but I'm not sure it is worth the readabilty penalty, and the harder overriding capabilities.
as a single multi-target derivation would need to be evaluated for every end platform and would duplicate perfectly agnostic Java bytecode for no reason.
I don't know if my explanation is clear...
You were clear, it's just that since the java compiler is different between the platforms, I suspect that the derivations won't get the same hash (which is based on their inputs).
There was a problem hiding this comment.
You were clear, it's just that since the java compiler is different between the platforms, I suspect that the derivations won't get the same hash (which is based on their inputs).
When I wrote it, I was thinking more of optimised binary caches that de-duplicate based on content, and about future content-addressed derivations. This was based on the assumption that javac, not being a native compiler, would have produced identical bytecode irrespective of the "real" underlying platform.
How are you so sure that this protege-build is the same for all platforms?
I performed some experiments, and it turns out that you were right: something creeps into the compiled jars and makes them different for, apparently, no reason. On the bright side, optimised stores can properly detect duplication even with differing directory structures, so all JAR dependencies are correctly hard-linked.
At this point, the only gain from the split derivation would be a reduced dependency set. I don't think it's worth it anymore. I will simplify the derivation and resubmit it shortly.
Thank you!
P. S. It would be very interesting to investigate the sources of impurity in Java bytecode, so that we can aim at reproducible Java programs in the future. I don't know if anyone has ever made some research in that direction, but I find it likely.
There was a problem hiding this comment.
P. S. It would be very interesting to investigate the sources of impurity in Java bytecode, so that we can aim at reproducible Java programs in the future. I don't know if anyone has ever made some research in that direction, but I find it likely.
That's something to mention at #313216 .
Thank you!
You are welcome!
On the bright side,
optimised stores can properly detect duplication even with differing directory structures, so all JAR dependencies are correctly hard-linked.
What is this optimise tool you are talking about?
There was a problem hiding this comment.
nix-store --optimise
|
BTW those maven patches are impressive :) Is there a chance you'd have an idea how to help here: ? |
|
Thank you~! They're nothing special really. Ok, I started looking into it. I'll report back under the relevant issue as soon as I have any positive outcomes (or negative outcomes different from the ones you obtained). |
| ''; | ||
| maintainers = with lib.maintainers; [ nessdoor ]; | ||
| license = with lib.licenses; [ bsd2 ]; | ||
| # TODO Protege is able to run on Darwin as well, but I (@nessdoor) had no |
There was a problem hiding this comment.
Given that, I would suggest to set platforms to unix and broken = stdenv.isDarwin.
There was a problem hiding this comment.
Given that, I would suggest to set platforms to
unixandbroken = stdenv.isDarwin.
Further more, we can use platforms.unix, and see what ofborg's CI says. If it will fail on Darwin, we can set meta.broken and even quote in a comment an excerpt from the error we got from there, so Darwin users who will want to unbreak it will have a clue what they have to tackle with.
There was a problem hiding this comment.
Let's see how ofBorg behaves, but I see no reason for it failing in any obvious way. I think Darwin users would just find themselves with a bogus package that needs to be started directly from the store path, as the corresponding startup script is not getting copied into /bin (if that's where today's Darwin systems expect to find user applications, that is).
There was a problem hiding this comment.
Let's see how ofBorg behaves, but I see no reason for it failing in any obvious way. I think Darwin users would just find themselves with a bogus package that needs to be started directly from the store path, as the corresponding startup script is not getting copied into
/bin(if that's where today's Darwin systems expect to find user applications, that is).
Yes ideally Darwin users would want to put something in $out/Applications, but it's not so bad for them as a start to have something only in $out/bin.
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
and removed
10.rebuild-darwin: 0
|
Result of 1 package failed to build:
|
But in the 2nd attempt it didn't fail... Something there is a bit flaky I suppose. Other then that GUI seemed to work fine. |
|
|
||
| nativeBuildInputs = [ | ||
| copyDesktopItems | ||
| iconConvTools |
There was a problem hiding this comment.
This package is not supported on Darwin, making the build not even evaluate there (I don't mean to request to fix this issue, just pointing out this fact after seeing ofborg logs for x86_64-darwin).
There was a problem hiding this comment.
I guessed as much, since the icon format on Darwin is different
|
Successfully created backport PR for |
|
Thank you very much for the extensive review comments, @AndersonTorres, @doronbehar! By the way, that (#316647 (comment)) failure sounds very weird. Seems like a concurrency problem in the tests? Well, but if the CI reported no issue, it means that everybody is going to get a correct package. |
Usually Hydra and perhaps also ofborg are more prone to concurrency problems then normal users' machines. Worst case we'll modify things in a later PR if the build fails on Hydra. Also it's possible that it will fail on Hydra and it won't fail locally, so users may not notice this issue.
You are welcome :). |
Description of changes
The current
protege-distributionpackage is derived directly from the upstream release archives, which include a number of pre-installed third-party plugins.This PR introduces the "vanilla" Protégé build with no added plugins. More importantly, it is a source-based build, contrary to the current "distribution" build, and is better engineered overall.
This Protégé build is platform-independent and the derivation has been structured to support both Linux and Darwin, but, due to the lack of a Darwin machine at hand, I could not come up with a working installation for Darwin systems. Input from knowledgeable people is welcome and solicited.
I will try to refactor the old
protege-distributionderivation into a source-based build as soon as technically possible, and maybe integrate it into the new derivation as a special case (maybe as aprotegeFullpackage?).Thanks go to @doronbehar for pointing me to
maven.buildMavenPackageas a way to properly build Maven projects.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.