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

marvin: init at 19.1.0 #53839

Merged
merged 1 commit into from Jan 19, 2019
Merged

marvin: init at 19.1.0 #53839

merged 1 commit into from Jan 19, 2019

Conversation

fusion809
Copy link
Contributor

@fusion809 fusion809 commented Jan 12, 2019

Motivation for this change

The Marvin Suite is one of the finest skeletal structure drawing programs that is available free of cost to the end user. There are some features one must pay for, but most of them are advanced features that many users, like myself, don't really care about. Here's a screenshot showing it running, opened to a molecule of clozapine:

screenshot_20190112_220048

. I have added myself to the maintainers list, as this package, of course, needs a maintainer.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Area that could do with some improvement

Line 9 of the default.nix file could do with some improvement. Namely, if anyone knows a way I could let the nix file determine the version 19.1 from the full version 19.1.0, and use it in the URL, it'd be much appreciated :). I've tried using ${version%.?} and sadly, it merely caused Nix to give the error message:

error: syntax error, unexpected $undefined, expecting '}', at /home/fusion809/GitHub/mine/packaging/nixpkgs/pkgs/applications/science/chemistry/marvin/default.nix:9:77

@fusion809 fusion809 changed the title marvin: init at 19.1 marvin: init at 19.1.0 Jan 12, 2019
@fusion809
Copy link
Contributor Author

fusion809 commented Jan 12, 2019

Ticking the "other Linux distributions" box as I have now tried it on Arch Linux and it ran fine.

mkdir $out/share/pixmaps
mkdir $out/bin
ln -sf $out/opt/chemaxon/marvinsuite/bin/msketch $out/bin/msketch
ln -sf $out/opt/chemaxon/marvinsuite/bin/mview $out/bin/mview
Copy link
Member

Choose a reason for hiding this comment

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

This does not work in an empty PATH.

 PATH= $buildInputs/bin/mview 
/nix/store/jb299sb3abjqfh4zm7qpkd6ir0jl8jfc-marvin-19.1.0/bin/mview: line 362: basename: No such file or directory
/nix/store/jb299sb3abjqfh4zm7qpkd6ir0jl8jfc-marvin-19.1.0/bin/mview: line 363: dirname: No such file or directory
/nix/store/jb299sb3abjqfh4zm7qpkd6ir0jl8jfc-marvin-19.1.0/bin/mview: line 378: dirname: No such file or directory
/nix/store/jb299sb3abjqfh4zm7qpkd6ir0jl8jfc-marvin-19.1.0/bin/mview: line 379: basename: No such file or directory
No suitable Java Virtual Machine could be found on your system.
The version of the JVM must be at least 1.8.
Please define INSTALL4J_JAVA_HOME to point to a suitable JVM.

It would be preferable to wrap these executables with the jre (and coreutils apparently). random example:

wrapProgram "$out/bin/$cmd" \
--prefix PERL5LIB : "$PERL5LIB" \
--prefix PATH ":" ${stdenv.lib.makeBinPath [
"$out" which libcdio-paranoia cddiscid wget
vorbis-tools id3v2 eyeD3 lame flac glyr
]}

Or if running a jar is enough:
makeWrapper ${jre}/bin/java $out/bin/astah \
--add-flags "-jar $out/share/astah/astah-community.jar"

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, although I must admit I'm a Nix beginner and Marvin has a whole directory of Java .jar files it uses, so your second suggestion (which I am able to follow) may not be applicable. Here's the contents of opt/chemaxon/marvinsuite: https://gist.github.com/78dcbe723995f9ab858f85db599c52f6. msketch has these contents: https://paste2.org/kEd9zFtL. It seems to be a shell wrapper in itself.

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 executing command, when the variables are expanded, is incredibly complex, e.g. here is what it is when I've extracted it to a local directory at /home/fusion809/Programs/Deb/marvin: https://gist.github.com/fusion809/188669fd39c100a2cfadc42c72774fb4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh thank goodness, I figured it out! Just had to find the docs for makeWrapper.

Copy link
Member

Choose a reason for hiding this comment

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

It still does not work for me in an empty PATH.

[nix-shell:~/.cache/nix-review/pr-53839]$ PATH= $buildInputs/bin/mview
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 362: basename: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 363: dirname: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 378: dirname: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 379: basename: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 66: expr: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 67: expr: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 81: awk: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 82: rm: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 83: mv: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 85: sed: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 88: chmod: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 66: expr: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 67: expr: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 81: awk: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 82: rm: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 83: mv: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 85: sed: No such file or directory
/nix/store/vs9h3hws3slbrliryxi80abmf48dws4j-marvin-19.1.0/opt/chemaxon/marvinsuite/bin/mview: line 88: chmod: No such file or directory
No suitable Java Virtual Machine could be found on your system.
The version of the JVM must be at least 1.8.
Please define INSTALL4J_JAVA_HOME to point to a suitable JVM.

You probably need to add something like --prefix PATH : ${makeBinPath [ coreutils awk sed ]}.
And the INSTALL4J_JAVA_HOME thing does not seem to work.
By the way, I find strange that a startup wrapper needs rm...

Copy link
Member

Choose a reason for hiding this comment

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

makeBinPath is in lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing makeBinPath with stdenv.lib.makeBinPath makes the build fail, with the error:

error: undefined variable 'coreutils' at /data/GitHub/mine/packaging/nixpkgs.marvin-pr/pkgs/applications/science/chemistry/marvin/default.nix:21:142

Sorry, I'm sure these are real stupid concerns, but I'm a beginner and I do not know why this is happening.

Copy link
Member

Choose a reason for hiding this comment

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

coreutils needs to be added to the arguments in the first line.

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, I do believe I have fixed it. I tested with an empty PATH and it launched.

Copy link
Member

Choose a reason for hiding this comment

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

nice !

buildInputs = [ dpkg makeWrapper ];
unpackPhase = ''
mkdir pkg/opt -p
dpkg-deb -x $src pkg/opt
Copy link
Member

Choose a reason for hiding this comment

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

you can directly unpack to $out/opt if you wish so.

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, done in 1e14e08.

@fusion809 fusion809 force-pushed the marvin branch 4 times, most recently from 2be61d3 to 355f8d8 Compare January 12, 2019 15:58
@fusion809
Copy link
Contributor Author

fusion809 commented Jan 12, 2019

In my last commit 355f8d8, I added extra wrappers to $out/bin, for all executables in $out/opt/chemaxon/marvinsuite, so:

bin/cxcalc
bin/cxtrain
bin/evaluate
bin/molconvert
bin/msketch
bin/mview
LicenseManager
MarvinSketch
MarvinView

I also created a desktop config file for the ChemAxon License Manager.

makeWrapper $out/opt/chemaxon/marvinsuite/bin/evaluate $out/bin/mview --set INSTALL4J_JAVA_HOME "${jre}"
makeWrapper $out/opt/chemaxon/marvinsuite/bin/molconvert $out/bin/mview --set INSTALL4J_JAVA_HOME "${jre}"
makeWrapper $out/opt/chemaxon/marvinsuite/bin/msketch $out/bin/msketch --set INSTALL4J_JAVA_HOME "${jre}"
makeWrapper $out/opt/chemaxon/marvinsuite/bin/mview $out/bin/mview --set INSTALL4J_JAVA_HOME "${jre}"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You can probably write a loop like

for i in cxcalc cxtrain etc; do
   makeWrapper $out/opt/chemaxon/marvinsuite/bin/$i $out/bin/$i --set INSTALL4J_JAVA_HOME "${jre}";
done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 1cdba3e048c.

@fusion809 fusion809 force-pushed the marvin branch 2 times, most recently from 1cdba3e to f6bb31a Compare January 12, 2019 18:12
Also adding myself (fusion809) as a maintainer.
The marvin Nix file in this commit is largely thanks to @msteen.
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

4 participants