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

PKGBUILD for snpeff ver 5.1 #100

Closed
wants to merge 16 commits into from

Conversation

kbipinkumar
Copy link
Member

Involved packages

  • snpeff

Details

  • Tested in the local machine (largest 16G RAM) and it is passed without any issue
  • Provide New Package
  • Fix the Packages
    • PKGBUILD
    • lilac.yaml
    • lilac.py
  • Would like to continue to work with us

Additional Note

the package provides both snpEff and SnpSift binaries.

@starsareintherose
Copy link
Member

I would suggest you complete the lilac.yaml just like what is written in tnt.

If this is package is maintained by yourself on AUR, add update_aur_repo() function.

For java package, better to compile from source unless it's difficult. I am still trying to fix current java applications. see my efforts on #85

@starsareintherose
Copy link
Member

starsareintherose commented Dec 1, 2022

try mvn package and see target or somewhere.

@kbipinkumar
Copy link
Member Author

kbipinkumar commented Dec 1, 2022 via email

@starsareintherose
Copy link
Member

just try mvn package command to check if it will fail or not. Usually command is mvn package (pom.xml) or ant dist (build.xml)

@kbipinkumar
Copy link
Member Author

I would suggest you complete the lilac.yaml just like what is written in tnt.

If this is package is maintained by yourself on AUR, add update_aur_repo() function.

For java package, better to compile from source unless it's difficult. I am still trying to fix current java applications. see my efforts on #85

just try mvn package command to check if it will fail or not. Usually command is mvn package (pom.xml) or ant dist (build.xml)

okay. i will give it a try

@hubutui
Copy link
Contributor

hubutui commented Dec 1, 2022

You might check also this doc.

source=(
snpEff.sh
SnpSift.sh
https://snpeff.blob.core.windows.net/versions/snpEff_latest_core.zip
Copy link
Member

@sukanka sukanka Dec 1, 2022

Choose a reason for hiding this comment

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

I recommend to use https://github.com/pcingola/SnpEff/archive/refs/tags/v${pkgver}.tar.gz,
and compile from source.

BTW, the latest version is 5.1d, see https://github.com/pcingola/SnpEff/releases/tag/v5.1d

Copy link
Contributor

@hubutui hubutui Dec 1, 2022

Choose a reason for hiding this comment

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

It's not a good version number, vercmp 5.1d 5.1 returns -1, which means 5.1d < 5.1. The upstream might need to use a proper version number.

Copy link
Member Author

Choose a reason for hiding this comment

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

I recommend to use https://github.com/pcingola/SnpEff/archive/refs/tags/v${pkgver}.tar.gz, and compile from source.

BTW, the latest version is 5.1d, see https://github.com/pcingola/SnpEff/releases/tag/v5.1d

That seems to be source code release. However the project main page as well as binaries lists version as 5.1

Copy link
Member

Choose a reason for hiding this comment

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

That seems to be source code release.

Whenever possible, compile from source.

However the project main page as well as binaries lists version as 5.1

This means the project main page is out-of-date and should be updated.

BTW, as the version number is not properly set, you may file an issue in the upstream first.

Copy link
Contributor

Choose a reason for hiding this comment

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

why not confirm with upstream? It would be a problem if the source and the binary release did not match.

@kbipinkumar
Copy link
Member Author

just try mvn package command to check if it will fail or not. Usually command is mvn package (pom.xml) or ant dist (build.xml)

so according to the doc biojava-core and biojava-structure are also dependencies for building snpeff from source

@starsareintherose
Copy link
Member

just try mvn package command to check if it will fail or not. Usually command is mvn package (pom.xml) or ant dist (build.xml)

so according to the doc biojava-core and biojava-structure are also dependencies for building snpeff from source

already in pom.xml https://github.com/pcingola/SnpEff/blob/master/pom.xml#L102

@hubutui
Copy link
Contributor

hubutui commented Dec 2, 2022

you might also make sure the files' permission is set to 644. I don't think they need x permission.

@kbipinkumar kbipinkumar marked this pull request as draft December 10, 2022 05:44
@kbipinkumar
Copy link
Member Author

kbipinkumar commented Dec 16, 2022

just try mvn package command to check if it will fail or not. Usually command is mvn package (pom.xml) or ant dist (build.xml)

so according to the doc biojava-core and biojava-structure are also dependencies for building snpeff from source

already in pom.xml https://github.com/pcingola/SnpEff/blob/master/pom.xml#L102

i have been trying to compile this package from sources. however, the sources for two utilities of the package SnpEff and SnpSift are hosted in two different github repositories as per the documentation from upstream as shown below.

# Get SnpEff
git clone https://github.com/pcingola/SnpEff.git

# Get SnpSift as well
git clone https://github.com/pcingola/SnpSift.git

i managed to write a PKGBUILD that can download source tar files from both repositories.
https://gist.github.com/kbipinkumar/24a3b653e5a79aa41562902c221b74dc
with this PKGBUILD i was able to successfully compile SnpEff and SnpSift. please take a look and let me know.

@kbipinkumar kbipinkumar marked this pull request as ready for review December 16, 2022 19:20
@sukanka
Copy link
Member

sukanka commented Dec 17, 2022

i managed to write a PKGBUILD that can download source tar files from both repositories. https://gist.github.com/kbipinkumar/24a3b653e5a79aa41562902c221b74dc with this PKGBUILD i was able to successfully compile SnpEff and SnpSift. please take a look and let me know.

Looks fine to me, but there are minor issues you should pay attention to.

According to Java_package_guidelines, java-environment instead of jdk-openjdk should be used.

And since nothing is done in prepare(), it can be removed.

BTW, since the two components share the same version number, I now think 5.1 is better than 5.1d. (SnpSift has no 5.1d tag, so 5.1 is fine.)

@kbipinkumar
Copy link
Member Author

kbipinkumar commented Dec 17, 2022

i managed to write a PKGBUILD that can download source tar files from both repositories. https://gist.github.com/kbipinkumar/24a3b653e5a79aa41562902c221b74dc with this PKGBUILD i was able to successfully compile SnpEff and SnpSift. please take a look and let me know.

Looks fine to me, but there are minor issues you should pay attention to.

According to Java_package_guidelines, java-environment instead of jdk-openjdk should be used.

And since nothing is done in prepare(), it can be removed.

BTW, since the two components share the same version number, I now think 5.1 is better than 5.1d. (SnpSift has no 5.1d tag, so 5.1 is fine.)

i have made the modifications as suggested as well as adding shell wrapper for jar files. please check and let me know if anything else is to be done.

@hubutui
Copy link
Contributor

hubutui commented Dec 17, 2022

  1. MIT license is not listed in licenses pkg, so we should install it to /usr/share/licenses/snpeff.
  2. PKGBUILD, lilac.py, lilac.yaml should be change to 644 permission.
  3. maybe remove the suffix of file snpEff.sh and snpSift.sh? I'm not sure if all lower-case is preferred.
  4. It seems there is a unused file SnpSift.sh?
  5. Could you git rebase and squash all these commits into one?

@kbipinkumar
Copy link
Member Author

  1. Could you git rebase and squash all these commits into one?

while attempting to squash commits i am getting following error.

error: could not apply f075eca39... Rewrote PKGBUILD to compile from source
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
**Could not apply f075eca39... Rewrote PKGBUILD to compile from source**

commit f075eca is the current PKGBUILD that compiles from source.

any ideas?

if nothing works, this pull request can be closed and new one be opened as snpeff has already been added ot Bioarchlinux:master branch. the rewritten PKGBUILD cn be submitted as fresh PR

@hubutui
Copy link
Contributor

hubutui commented Dec 18, 2022

I think you could update the PKGBUILD to the file we wanted, and git add PKGBUILD, and git rebase --continue, then finally git push -f to your fork.

@hubutui
Copy link
Contributor

hubutui commented Dec 18, 2022

Oh, it seems you fork this repo, and work on a branch snpeff which also merge https://github.com/kbipinkumar/Packages_sub/commit/fe5f5ff899acd9b1602adaa7bed5d0bc3779868d to the branch, which make it complicated.

It would be much easier to:

  1. rebase your forked repo https://github.com/kbipinkumar/Packages_sub to this repo, which could be done as the doc describe.
  2. create another branch from the updated forked repo, update the files we need, commit, and push it to the new branch in your forked repo.
  3. create another pull request.

Maybe you could use the same branch name in the forked repo, and then you don't need to create another pull request. If you choose this method, remember to back up your files before deleting the old branch so that you could create a new branch with the same name.

@hubutui
Copy link
Contributor

hubutui commented Dec 18, 2022

another tip, LF line terminator is preferred. It seems lilac.yaml is with CRLF line terminators.

@kbipinkumar kbipinkumar deleted the snpeff branch December 18, 2022 08:08
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.

4 participants