Skip to content

Script that uploads distribution packages to Bintray.#1002

Merged
maciejwalkowiak merged 5 commits into
mainfrom
gh-952
Oct 23, 2020
Merged

Script that uploads distribution packages to Bintray.#1002
maciejwalkowiak merged 5 commits into
mainfrom
gh-952

Conversation

@maciejwalkowiak

Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Script that uploads distribution packages to Bintray.

The script is written in Kotlin and just outputs the commands that have to be executed with bash. The command itself was tested with my test package but must be verified with Sentry Bintray.

By default, script uploads packages but does not publish them - which can be changed later once we get confidence that this process works.

💡 Motivation and Context

Fixes #952.

💚 How did you test it?

Maven command has been tested with my test package.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

  1. @bruno-garcia or @marandaneto verify if these commands work with Sentry packages
  2. Write github action that downloads packages from Craft and then execute the script.

@bruno-garcia bruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

Comment thread scripts/release.kts Outdated
Comment thread scripts/release.kts Outdated
val sourcesFile = "$path/$module-sources.jar"
val pomFile = "$path/pom-default.xml"

val command = "mvn deploy:deploy-file -Dfile=$file -Dfiles=$javadocFile,$sourcesFile -Dclassifiers=sources,javadoc -Dtypes=jar,jar -DpomFile=$pomFile -DrepositoryId=$repositoryId -Durl=$repositoryUrl\\;publish\\=$publish --settings $settingsPath"

@marandaneto marandaneto Oct 21, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this requires mvn installed which is fine, but afaik, bintray doesn't follow the mvn API conventions, unless they have something new, last time I've checked it, bintray has its own APIs and CLIs to upload stuff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AFAIK it works with Maven deploy-file - check SO question, I also tried it with my test project. We need to verify it with sentry-java to be sure.

Regarding Maven requirement, we can either expect Maven to be there or I can add Maven wrapper so that we will just call ./mvnw deploy:deploy-file ... and proper Maven version will get installed. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if it works I'm happy with it.

The maven wrapper would be great, so nobody needs to care about the mvn version, etc, good idea.

Right now we are signing the files directly on bintray using bintray's key and autosign feature enabled here:
https://github.com/getsentry/sentry-java/blob/main/sentry/build.gradle.kts#L96

I believe we'd need to sign all the files locally (even poms) before publishing to bintray, which is gonna create a few more files (*.asc) and those should be uploaded too.

Not sure if we can pass this option via deploy-file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can we give it a try? I think autosigning should work for Maven uploads too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yep we can try, but even on bintray's gradle plugin and bintray's CLI, this is opt-in, so I don't think it's gonna be enabled via deploy-file

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Comment thread scripts/release.kts Outdated
* To execute the script two environment variables that are used by Maven have to be present: BINTRAY_USERNAME, BINTRAY_API_KEY
*
* Example usage (assuming that the script is executed from the `<project-root>/scripts` directory and the distribution files are in `<project-root>/dist`):
* $ kotlinc -script release.kts -- -d ../dist -s -repositoryUrl https://api.bintray.com/maven/sentry/sentry-java/sentry-java/ | sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we have a compiled CLI of it? otherwise, it'll require kotlinc in the path, but fine with it for now.

also, important to note that repositoryUrl changes per project type (Android or Java)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added two properties for both repositories. We could have compiled but I thought it's just more convenient to have a script in sources..?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will ultimately run on a service so we can make kotlinc be a install requirement, prob best to be editable/script at this point

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah I just wanted to make it easier, even myself didn't have kotlinc on the system env path, I've always used Gradle so I needed to https://kotlinlang.org/docs/tutorials/command-line.html
but anyway, via brew is 2s so I'm in for it

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing changelog, other than that LGTM

@maciejwalkowiak maciejwalkowiak merged commit 0120409 into main Oct 23, 2020
@maciejwalkowiak maciejwalkowiak deleted the gh-952 branch October 23, 2020 07:45
@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

Changelog is meant only for changes that affect users.

@marandaneto

Copy link
Copy Markdown
Contributor

Changelog is meant only for changes that affect users.

good point, I wrote it when reading the CI checks, LGTM

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.

Figure out pushing a package to bintray via API

3 participants