Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protege: init at 5.5.0 #97365

Closed
wants to merge 1 commit into from
Closed

Conversation

mucaho
Copy link
Contributor

@mucaho mucaho commented Sep 7, 2020

Motivation for this change

Adds Protege, a free, open-source ontology editor and framework for building intelligent systems.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
    Not applicable here, not a nixos module
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
    It's a new application, no packages do or will depend upon it
  • Tested execution of all binary files (usually in ./result/bin/)
    Tested also using runtime-sandboxing (nix-shell -I nixpkgs=. --pure -p protege)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
    Couldn't run the command due to error, but it's around 60MB large
    error: Please be informed that this pseudo-package is not the only part of Nixpkgs that fails to evaluate. You should not evaluate entire Nixpkgs without some special measures to handle failing packages, like those taken by Hydra.
  • Ensured that relevant documentation is up to date
    No documentation needed here
  • Fits CONTRIBUTING.md.
    Yes, except for the maintainers field. Happy to take a look and update it from time to time, but can't fully commit as maintainer.

pname = "protege";
version = "5.5.0";

src = fetchzip {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not absolutely against using this zip file, but there's a new part of the manual explaining how to build maven projects in nixpkgs from source:

https://nixos.org/manual/nixpkgs/unstable/#maven

Could you please give it a try?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review!
I didn't have time to try the maven packaging yet, however @JonathanReeve would like to start using the package using the current solution proposed by @mgttlinger .
Could we maybe merge this version if it's ok and refine upon it on the future?

Copy link
Contributor

@nessdoor nessdoor Jun 19, 2021

Choose a reason for hiding this comment

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

@doronbehar the "blessed" technique suggested in the manual apparently fails to detect the dependency for org.apache.felix:maven-bundle-plugin. As the translation from pom.xml to project-info.json is not fully clear to me, I also went for a direct JAR package.

@doronbehar
Copy link
Contributor

There's merge conflicts.

@JonathanReeve
Copy link
Contributor

I'd love to see this package included. What can be done to fix it up? One thing is, I notice it doesn't build unless I also include appimagekit, which provides the `desktop-file-validate' binary it needs.

But then it seems to give lots of errors while running it, so I'm not sure what's going on here.

@JonathanReeve
Copy link
Contributor

@mucaho, could you take another look at this, when you get a chance? I'd love to be able to use this package, but I don't know enough about Java packaging to get this to work.


meta = {
description = "Ontology editor and framework for building intelligent systems";
license = lib.licenses.bsd2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Protege itself is released under a 2-clause BSD, but this is the distribution version, which includes third-party plugins. These come with a variety of licenses that can be extracted from the individual JARs.

From what I could see, there are pieces that come under Apache 2.0, Eclipse Public License 1.0 and LGPL 3.0 (don't know if in the "only" or "or later" form, though). It may be good to include these additional licenses here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The official github protege distribution license is BSD-2 https://github.com/protegeproject/protege-distribution .
Putting other licenses in the meta field makes it sound like the user may choose which license terms they accept, which is not the case here.

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the relevant manual section, a list is used to indicate that different parts of the package are licensed differently. I agree that this can be confusing, but the solution would be to improve the meta-attribute themselves.

Also, as far as I understand the license field is used by binary caches to determine if a package can be cached and redistributed. If one of the licenses is "unfree", then I suppose the whole package would be marked "uncacheable" as to comply to licensing regulations.

@@ -25161,6 +25161,8 @@ in

protonmail-bridge = callPackage ../applications/networking/protonmail-bridge { };

protege = callPackage ../applications/science/logic/protege { };
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if Protege should be lumped together with things like theorem provers, and it's more of a development/content creation tool than a scientific instrument. The base application doesn't even include a reasoner, for instance.

But this is just a matter of opinions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, putting it here was unintentional.

@SuperSandro2000
Copy link
Member

Closing in favor of #127396

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

Successfully merging this pull request may close these issues.

None yet

6 participants