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

LUCENE-9488 Assemble source tar, with checksum and signing #353

Merged
merged 4 commits into from Oct 5, 2021

Conversation

janhoy
Copy link
Contributor

@janhoy janhoy commented Oct 5, 2021

https://issues.apache.org/jira/browse/LUCENE-9488

Creates the source tar, with checksum and gpg signing. Usage:

./gradlew -Dversion.release=9.0.0 assembleSourceDist
ls -l lucene/packaging/build/distributions 
./gradlew -Psigning.gnupg.keyName=0D8D0B93 -Dversion.release=9.0.0 signSourceDistTar
ls -l lucene/packaging/build/distributions 

@janhoy janhoy requested a review from dweiss October 5, 2021 19:34
@janhoy
Copy link
Contributor Author

janhoy commented Oct 5, 2021

First attempt on a solution, and it works. But I'm pretty sure there is a more optimal organization of the code, i.e. the task definition could perhaps live somewhere else.

Also, the sha512 hash generation is a raw copy/paste of some lines from elsewhere, probably some smart way to factor that out and make it DRY. Happy for suggestions.

@dweiss
Copy link
Contributor

dweiss commented Oct 5, 2021

It's quite terrible gradle code, but it works! :) I will get back to clean this up some day, promise. I just don't have any ready to use solutions to copy from - the projects I have worked on just signed/ distributed maven artifacts, that's it.

I filed LUCENE-10152 to look into this one day. For the moment being, just feel free to move ahead.

def sourceTarFile = file("build/distributions/lucene-${version}-src.tgz")
import org.apache.commons.codec.digest.DigestUtils

task assembleSourceDist() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be typically an internal task attached to convention-style assemble:

assemble.dependsOn assembleSourceDist

Since we're using an external tool (git) and the whole process may be costly, I wouldn't worry about this.

lucene/packaging/build.gradle Outdated Show resolved Hide resolved
task assembleSourceDist() {
def target = sourceTarFile

outputs.files target
Copy link
Contributor

Choose a reason for hiding this comment

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

An input to this task could be the head commit's sha... this way it wouldn't re-run archiving of the same commit. Again - not important for now.

checksum(sourceTarFile)
}
}
tasks.findByName("assembleDist").dependsOn(assembleSourceDist)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's 'assembleDist'? Where is it defined? findByName is not needed - if it's a known task, you can just say

assembleDist.dependsOn(assembleSourceDist)

but I can't find this task anywhere.

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 think it is defined by the distribution plugin. It will by default do distTar and distZip. I'll simplify the line..

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah... right - can be. I never used this plugin, so I'm not familiar with it.

@janhoy janhoy merged commit 5cd0d68 into apache:main Oct 5, 2021
@janhoy janhoy deleted the lucene9488-assembleSourceDist branch October 5, 2021 20:30
@dweiss
Copy link
Contributor

dweiss commented Oct 6, 2021

Thanks @janhoy !

@dsmiley
Copy link
Contributor

dsmiley commented Oct 24, 2021

I'm looking at this and seeing gradle/maven/publications-maven.gradle at the bottom has:

  tasks.withType(GenerateModuleMetadata) {
    enabled = false
  }

Added in this PR by @dweiss shown by git blame.
Why? This means if we use more sophisticated metadata like feature-variants (which I'm working on now for spatial-extras), it will not be published, which is a shame.

@dweiss
Copy link
Contributor

dweiss commented Oct 25, 2021

Because back in the day it didn't work for me and broke downstream maven builds. If you'd like to check whether things work without it, please remove it and file a pr. I don't care.

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.

None yet

3 participants