Skip to content

new package Rscape-2.0.0-k #5344

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

Merged
merged 4 commits into from
Aug 24, 2022
Merged

new package Rscape-2.0.0-k #5344

merged 4 commits into from
Aug 24, 2022

Conversation

marcom
Copy link
Contributor

@marcom marcom commented Aug 23, 2022

R-scape is a package for predicting RNA secondary structure using evolutionary covariation observed from multiple sequence alignments.

http://rivaslab.org/
http://eddylab.org/R-scape/

Comment on lines 6 to 12
version = v"2.0.0-k"

file_version_str = if isempty(version.prerelease)
"$version"
else
"$(version.major).$(version.minor).$(version.patch).$(first(version.prerelease))"
end
Copy link
Member

Choose a reason for hiding this comment

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

We can't really use prerelease tags in version numbers of packages, maybe one day we'll explicitly prohibit it. I usually do the other way around: write down the version string used by upstream, and build up the version we want to use out of it, maybe something like

VersionNumber(replace(file_version_str, r"([0-9]+\.[0-9]+\.[0-9]+).*" => s"\1"))

?

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 for the review!

I made the change, though i have to admit i think it would be nice to use the correct version numbers for binary packages. Not sure if that would be have to be done at the Pkg or the BinaryBuilder level. Or maybe there is something i am missing :)

Copy link
Member

@giordano giordano Aug 24, 2022

Choose a reason for hiding this comment

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

It's a limitation of Pkg, it doesn't really like anything outside of X.Y.Z versioning scheme, with JLL packages we're already outside of it with the use of the build number, which causes some issues. Also, lots of projects have very wild versioning schemes, completely outside of the X.Y.Z-prerelease+build scheme, we can't possibly cater for all of them so we have to do some normalisation anyway.

Comment on lines 143 to 146
ExecutableProduct("MetamakeDemos.pl", :MetamakeDemos_pl),
ExecutableProduct("pdb_parse.pl", :pdb_parse_pl),
ExecutableProduct("r2r_msa_comply.pl", :r2r_msa_comply_pl),
ExecutableProduct("SelectSubFamilyFromStockholm.pl", :SelectSubFamilyFromStockholm_pl),
Copy link
Member

Choose a reason for hiding this comment

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

Note that usually we use ExecutableProduct for binary executables (ELF/COFF/MachO files), not interpreted scripts, because they aren't standalone (modulo required libraries) and need an interpreter to actually run. FileProduct should be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the change, and also removed the MetamakeDemos.pl script as it needed the source tarball directory to work.

I guess there could be some difficulty in using the FileProduct scripts in the case that they in turn call ExecutableProducts, as one would have to manually set up the environment (setenv) like it is done for an ExecutableProduct. I eyeballed that this is not the case here.

Just for future reference, if that were to be the case (script calling executable installed by _jll), would one then use ExecutableProduct or still choose FileProduct?

Copy link
Member

Choose a reason for hiding this comment

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

Just for future reference, if that were to be the case (script calling executable installed by _jll), would one then use ExecutableProduct or still choose FileProduct?

Eh, good question. Maybe we could do an exception for those cases, but it remains the problem that you may not be able to run the script if you're missing the interpreter at the expected path (Windows being the usual suspect)

marcom and others added 2 commits August 24, 2022 00:27
- perl scripts now use FileProduct instead of ExecutableProduct

- remove MetamakeDemos.pl from products, it needs the source directory

Co-authored-by: Mosè Giordano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants