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

Add jitpack support for SNAPSHOTS #5056

Merged
merged 5 commits into from
Aug 16, 2018

Conversation

magicDGS
Copy link
Contributor

Closes #4819

@magicDGS
Copy link
Contributor Author

@lbergelson - maybe you can have a look, as you commented in the original issue. As the commit isn't in the repo yet, I cannot test if it is working here. But here are the logs for the commit in the repo where I did the change: https://jitpack.io/com/github/bioinformagik/gatk/6270fa279/build.log

@codecov-io
Copy link

codecov-io commented Jul 26, 2018

Codecov Report

Merging #5056 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #5056      +/-   ##
============================================
+ Coverage     86.35%   86.37%   +0.02%     
- Complexity    28824    29156     +332     
============================================
  Files          1791     1814      +23     
  Lines        133601   135364    +1763     
  Branches      14920    15042     +122     
============================================
+ Hits         115364   116914    +1550     
- Misses        12834    12996     +162     
- Partials       5403     5454      +51
Impacted Files Coverage Δ Complexity Δ
...ender/tools/spark/sv/utils/GATKSVVCFConstants.java 0% <0%> (-75%) 0% <0%> (-1%)
...s/spark/ParallelCopyGCSDirectoryIntoHDFSSpark.java 0% <0%> (-74.257%) 0% <0%> (-17%)
...nder/tools/spark/pipelines/PrintVariantsSpark.java 0% <0%> (-66.667%) 0% <0%> (-2%)
...oadinstitute/hellbender/utils/test/XorWrapper.java 13.043% <0%> (-65.217%) 2% <0%> (-7%)
...utils/smithwaterman/SmithWatermanIntelAligner.java 50% <0%> (-30%) 1% <0%> (-2%)
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 54.194% <0%> (-25.806%) 30% <0%> (-10%)
...nder/tools/spark/BaseRecalibratorSparkSharded.java 0% <0%> (-22.807%) 0% <0%> (-2%)
...bender/tools/walkers/annotator/ReferenceBases.java 83.333% <0%> (-16.667%) 6% <0%> (+1%)
...titute/hellbender/utils/test/MiniClusterUtils.java 78.947% <0%> (-10.526%) 6% <0%> (-1%)
...org/broadinstitute/hellbender/utils/BaseUtils.java 86.726% <0%> (-3.54%) 46% <0%> (-2%)
... and 106 more

@lbergelson lbergelson self-assigned this Aug 7, 2018
Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

@magicDGS I have a few suggestions. Have you tested that this works or is that impossible until it's merged?

@@ -0,0 +1,11 @@
#!/bin/bash

GIT_LFS_VERSION="1.2.1"
Copy link
Member

Choose a reason for hiding this comment

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

I'd use a more recent version, maybe probably 2.5.1 since it's the most recent.

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


echo "Fetching LFS files."
$GIT_LFS install
$GIT_LFS pull
Copy link
Member

Choose a reason for hiding this comment

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

You should use git lfs pull --include src/main/resources/large which only downloads the ~50mb of files required to build gatk rather than the 3+ gb of test resources required to test it as well.

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

@@ -0,0 +1,4 @@
jdk:
Copy link
Member

Choose a reason for hiding this comment

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

If it's possible to add comments to this yaml, could you add one explaining what it is for people who aren't familliar with jitpack.

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 will first push the next changes to test the build (can also be tested from the fork, as this branch is not part of this repo) and then try to add the comments (jitpack have very bad documentation for the yml, I extracted the info from https://jitpack.io/docs/BUILDING/#custom-commands)

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 (only adding a purpose comment) - let me know if you would like more info

@@ -0,0 +1,11 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment to this file explaining what it is for people who don't know what jitpack is.

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

@lbergelson lbergelson assigned magicDGS and unassigned lbergelson Aug 14, 2018
@magicDGS
Copy link
Contributor Author

@lbergelson - the last commit works. Check the logs and artifacts in this folder: https://jitpack.io/com/github/bioinformagik/gatk/336d389ecc605f4138226dddda41981f78278e09

@magicDGS
Copy link
Contributor Author

Back to you @lbergelson - also, do you think that this will still work after #5112?

@lbergelson
Copy link
Member

@magicDGS It looks like jitpack runs ./gradlew install ( https://jitpack.io/docs/BUILDING/#gradle-projects ) which will create both the normal and testutils jars after that's split out. So it should be fine. You'll obviously have to change how you import the jars to include the test utils jar but there's no getting around that.

@lbergelson lbergelson merged commit b8bbb9c into broadinstitute:master Aug 16, 2018
@lbergelson
Copy link
Member

@magicDGS Thanks for doing this.

@magicDGS magicDGS deleted the dgs_jitpack_snapshots branch August 16, 2018 21:22
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.

3 participants