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

Make repository jitpack.io compatible #145

Merged
merged 2 commits into from Apr 21, 2021
Merged

Make repository jitpack.io compatible #145

merged 2 commits into from Apr 21, 2021

Conversation

mikehardy
Copy link
Contributor

@mikehardy mikehardy commented Apr 16, 2021

The repository was suprisingly close to working on jitpack

  • the ndkVersion jitpack uses is a little different
  • I personally bumped the version of the library itself for testing, that's bogus for the PR and I'll follow this with an annotation in the PR to change it
  • the javadoc task wasn't right and couldn't find symbols etc, I copy-pasted in a task from another library I maintain

Fixes #140

https://jitpack.io/com/github/mikehardy/sqlite-android/3.35.1.1/build.log

Takes about 10 minutes for jitpack to build the artifact starting from the first time someone requests it.

You can already depend on this artifact and try it if you like:

    implementation 'com.github.mikehardy:sqlite-android:3.35.1.1'

@@ -8,13 +8,13 @@ apply plugin: 'com.jfrog.bintray'
apply plugin: 'de.undercouch.download'

group = 'io.requery'
version = '3.34.1'
version = '3.35.1.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.

This is bogus for the purposes of the main repo. This repo is still on 3.35.1 so should be more like this:

Suggested change
version = '3.35.1.1'
version = '3.35.1'

...and then the commit that puts this PR in place should be tagged 3.35.1, with the tag pushed to github, in order for jitpack to build it

mikehardy added a commit to mikehardy/Anki-Android that referenced this pull request Apr 16, 2021
requery / sqlite-android was still using jcenter, which is going away

This is tracked upstream:
requery/sqlite-android#140
requery/sqlite-android#145
mikehardy added a commit to ankidroid/Anki-Android that referenced this pull request Apr 16, 2021
requery / sqlite-android was still using jcenter, which is going away

This is tracked upstream:
requery/sqlite-android#140
requery/sqlite-android#145
@npurushe
Copy link
Contributor

Awesome thanks for this!

@mikehardy
Copy link
Contributor Author

NP - thanks for keeping this thing going so long, and hopefully this makes that easier, as mentioned elsewhere I have a library in mavenCentral() and while that's cool and all it was time I didn't have in order to get it set up :-).

We have an alpha of AnkiDroid built using my forked-temporary-proof-of-concept dependency already and it all worked using GitHub Actions to do the build and release, so I'd say it's not a science fair project, it appears "good to go" to me, just requires your "release" process now to be tagging the code with a version number any time you want people to be able to use jitpack as their maven repo, and then (as a courtesy) you should probably try to fetch the tag once so that it builds :)

Heck, that's probably all doable in a github action as well - I think they can listen to push events, see if it is a tag, and then curl the .pom for the new tag to kick off the build, could sleep for 15 minutes, curl the build log and make sure it was a success?

If anyone is interested in hacking that together :-), otherwise just push a tag and will work...

@M66B
Copy link
Contributor

M66B commented Apr 17, 2021

@mikehardy thanks for contributing this, especially looking at the upcoming deprecation of Bintray.

@mikehardy
Copy link
Contributor Author

@M66B exactly why I was doing it :-), this was the last dependency I had stopping me from removing jcenter() maven repo in one of my apps, and I know open source repository maintainership is hard to move up on the priority list plus publishing on mavenCentral is a pain in the butt I was guessing @npurushe wasn't looking forward to ;-). With this it just needs to be merged (with correct version for next release), tagged with current sqlite version and...done

@M66B
Copy link
Contributor

M66B commented Apr 17, 2021

@npurushe since there were a number bugs fixed in the latest sqlite sub versions, so you might want to jump to version 3.35.4:

https://www.sqlite.org/changes.html

Let me thank you, like @mikehardy, for keeping this useful repository up to date! This doesn't mean there is a rush to update to the latest version now, although it would be welcome to me because I can finally remove some unused columns without doing a risky alterations of the database schema. This is not a big deal in any case, more a nice to have.

prateek-singh-3212 pushed a commit to prateek-singh-3212/Anki-Android that referenced this pull request Apr 19, 2021
requery / sqlite-android was still using jcenter, which is going away

This is tracked upstream:
requery/sqlite-android#140
requery/sqlite-android#145
@npurushe
Copy link
Contributor

I merged a prior PR that bumps the version to 3.35.4 and also removed the old bintray publish. Can you merge those changes into this? then will merge this PR thanks!

@mikehardy
Copy link
Contributor Author

Yes, and I just realized I need to update the docs as well (the README.md, that is)
I will merge those changes and an update to the docs so that it should be perfect for a 3.35.4 tag, test it on my fork, and if jitpack builds it right, it will be possible to just merge it here and tag it and we can all move on with our lives :-)

@mikehardy
Copy link
Contributor Author

I've made all the changes in one clean commit that should be ready to merge.

jitpack.io is still building the tag on my fork to confirm it's 💯 but it looks good so far - you should confirm prior to merge by making sure the log looks right

One "gotcha" with this is that if you forget to git push --tags to actually send the tag up to github.com, then jitpack.io can't see it to build it. So don't be like me and forget to push the tags then wonder why it's not building 🤡

Once the tag is up though, any attempt to access https://jitpack.io/com/github/requery/sqlite-android/<version> will trigger the build of <version> at which point you may actually watch it the build output in real time (takes about 10mins) at https://jitpack.io/com/github/requery/sqlite-android/<version>/build.log

Since I obviously have the URLs handy already, let's make that concrete and clickable

1- post merge make a tag of 3.35.1 however you prefer
2- click https://jitpack.io/com/github/requery/sqlite-android/3.35.4
3- click here to watch build https://jitpack.io/com/github/requery/sqlite-android/3.35.4/build.log
4- profit

@mikehardy
Copy link
Contributor Author

mikehardy commented Apr 19, 2021

Oh wait, scratch that. I wasn't expecting any of the changes from your previous merges to break things but something did.


* What went wrong:
Execution failed for task ':sqlite-android:publishMavenPublicationToMavenLocal'.
> Failed to publish publication 'maven' to repository 'mavenLocal'
   > Invalid publication 'maven': POM file is invalid. Check any modifications you have made to the POM file.

Investigating.

- reverts maven publishing changes, those break jitpack
- forward port 2 things to gradle7 (archiveClassifier + each->all)
@mikehardy
Copy link
Contributor Author

Okay, I had to revert your maven publishing changes @npurushe - apologies, they simply did not work for jitpack because it won't have the local properties file and it fails, plus something about that POM definition did not work. However, you can read the generated POM with that block commented out and it's got what you want in there...

This commit built successfully, and is ready for merge:

https://jitpack.io/com/github/mikehardy/sqlite-android/935649fe9d46f7229429a487e26819bedd022bfe/build.log

It includes documentation updates to use the jitpack reference, and I forward-ported the build file to gradle 7 for you ;-) (I just had to do that for AnkiDroid so you get my learnings free! what a bargain)

@@ -13,7 +13,7 @@ description = 'Android SQLite compatibility library'
android {
compileSdkVersion 30
buildToolsVersion "30.0.3"
ndkVersion '21.4.7075529'
ndkVersion '21.1.6352462'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

jitpack fails to find the ndk if you don't use the version they attempt, this is their version - from trial and error

@@ -107,19 +107,28 @@ if (localProperties.exists()) {
}

task sourceJar(type: Jar) {
classifier = 'sources'
archiveClassifier.set('sources')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gradle 7 compatibility

if (JavaVersion.current().isJava9Compatible()) {
options.addBooleanOption('html5', true)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the javadocs task did not work on jitpack before, but this one works

failOnError false
}

task javadocJar(type: Jar, dependsOn: javadoc) {
classifier = 'javadoc'
archiveClassifier.set('javadoc')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gradle 7 compatibility

connection = 'scm:git:git://github.com/requery/sqlite-android.git'
url = 'https://github.com/requery/sqlite-android'
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This caused ./gradlew :sqlite-android:publishMavenPublicationToMavenLocal to fail with invalid POM file. No idea why because if you comment it out and examine the POM it generates from defaults, it's exactly what you specify here and I'm certain I cleaned it so it was fresh and actually built like that. You can check my final jitpack build as well - https://jitpack.io/com/github/mikehardy/sqlite-android/935649fe9d46f7229429a487e26819bedd022bfe//sqlite-android-935649fe9d46f7229429a487e26819bedd022bfe.pom - so you don't have to trust me, you can see it :-)

pom.withXml {
asNode().children().last() + project.pomXml
def dependencies = asNode().appendNode('dependencies')
configurations.compile.allDependencies.each {
configurations.compile.allDependencies.all {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gradle7 compatibility - this has to do with when things are evaluated and all the "deferred configuration" stuff they are doing for performance. No semantic difference otherwise

password properties.getProperty('sonatype.password')
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

jitpack doesn't have a properties file with those values so it can't handle this

@mikehardy
Copy link
Contributor Author

I won't have any more time really to go back and forth on this so I annotated the full diff with explanations of everything. Hopefully it's good and can merge or I'll just use my fork for now. It's busy times handling the jcenter thing across a few ecosystems...

@npurushe
Copy link
Contributor

Sorry that change caused an issue with this PR i'll investigate what went wrong, thanks!

@mikehardy
Copy link
Contributor Author

Yeah, it seemed innocuous, I was surprised it did not just go first time!

@mikehardy
Copy link
Contributor Author

I just merged your changes from master in to here (I had simply commented the same lines you removed so took 2 secs) may still be good to go :-) ?

@npurushe
Copy link
Contributor

sounds good merging your PR to master and will try the jitpack version in a test app, thanks!

@npurushe npurushe merged commit 0b4d550 into requery:master Apr 21, 2021
@mikehardy
Copy link
Contributor Author

It looks good! https://jitpack.io/com/github/requery/sqlite-android/master-0b4d5508cc-1/build.log

just needs a tag on that commit hash and it's a good build, easy to integrate - I've test integrated with AnkiDroid from a build that did all the important (and otherwise untouched) bits (download + compile + link) the same so I've got high confidence in that part,

@npurushe
Copy link
Contributor

Great! added a tag for version 3.35.4

mikehardy added a commit to mikehardy/Anki-Android that referenced this pull request Apr 22, 2021
Upstream is now off jcenter officially after collaboration + merge of:
requery/sqlite-android#145
@mikehardy mikehardy deleted the release branch April 22, 2021 19:16
mikehardy added a commit to ankidroid/Anki-Android that referenced this pull request Apr 23, 2021
* Bump fragment from 1.3.2 to 1.3.3 (#320)
* Bump fragment-testing from 1.3.2 to 1.3.3 (#319)
* Bump mockito-inline from 3.8.0 to 3.9.0 (#318)
* chore: bump sqlite-android to 3.35.4, re-point upstream

Upstream is now off jcenter officially after collaboration + merge of:
requery/sqlite-android#145

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
mikehardy added a commit to mikehardy/Anki-Android that referenced this pull request May 14, 2021
requery / sqlite-android was still using jcenter, which is going away

This is tracked upstream:
requery/sqlite-android#140
requery/sqlite-android#145
mikehardy added a commit to mikehardy/Anki-Android that referenced this pull request May 14, 2021
* Bump fragment from 1.3.2 to 1.3.3 (#320)
* Bump fragment-testing from 1.3.2 to 1.3.3 (#319)
* Bump mockito-inline from 3.8.0 to 3.9.0 (#318)
* chore: bump sqlite-android to 3.35.4, re-point upstream

Upstream is now off jcenter officially after collaboration + merge of:
requery/sqlite-android#145

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@kierse kierse mentioned this pull request Aug 4, 2022
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.

Publish to Maven Central
3 participants