-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Commons: incorrect MavenPublication configuration #150
Comments
@tom93 check settings.gradle.kts in the the gallery. |
Thanks. I tried it on File-Manager but got some errors (e.g. in ChangeViewTypeDialog.kt, useForThisFolderDivider had type DividerBinding instead of View), sounds very similar to the binding issues you ran into in SimpleMobileTools/Simple-Gallery#2967. I don't have a clue why, do you have any insights? (Since the Maven local repository works, my take is that it's not worth changing the code to make includeBuild() work unless includeBuild() is uncovering an actual bug.) And is it okay if I still create a PR with my patch from above? The existing Maven configuration seem plain wrong. |
Yep, it's the same issue. I never investigated it, it probably has something to do with how bindings are generated.
Sure but please do test the Jitpack build once, you can trigger a build at https://jitpack.io/#org.fossify/commons or https://jitpack.io/#tom93/commons in your case. |
The configuration was incorrect. The JitPack build still worked, but publishing to the Maven local repository using `./gradlew publishToMavenLocal` was broken (incorrect paths and metadata), and composite builds didn't get automatic dependency substitution: building other apps against a local version of Commons using `./gradlew --include-build=../Commons ...` didn't work, and using `includeBuild("../Commons")` in <app>/settings.gradle.kts only worked with a manual substitution directive[1]. Also move project identity values out of gradle/libs.versions.toml. Detailed changes: - Fix the Maven groupId (it should be "org.fossify", not "org.fossify.commons"). - Set the Gradle project group. This allows automatic dependency substitution in composite builds (`--include-build=../Commons`). Also set the Gradle project version for consistency. - Use defaults for MavenPublication. (It's no longer necessary to explicitly set the Maven groupId/artifactId/version because they default to the Gradle project group/name/version. This also fixes the artifactId, which was previously "release" but should be "commons".) - Avoid using libs.versions.toml for project identity. (That file is normally used for dependencies. Using it to set the project's own name and version is unusual. Also, it's confusing to have the "samples" project's appId and versionCode mixed together with the library things.) - Move android.namespace declaration up (it's more logical and matches the official examples). - Tweak the include() call in settings.gradle.kts (reorder so that "commons" is first because it's more important than "samples", and make colons consistent). Fixes FossifyOrg/General-Discussion#150. [1] https://github.com/FossifyOrg/Gallery/blob/692a5d08b30e7d4d374f53d7371781d05b482dc9/settings.gradle.kts#L26
@naveensingh I created a PR (FossifyOrg/Commons#46) with an improved version of the patch from above that also fixes composite builds (and moves the values out of gradle/libs.versions.toml). I'm not sure how to trigger the JitPack build (it asks me to log in), can you please check for me? The commit is Aside: I found and wrote a patch for a serious issue in the Messages app relating to deletion, please give that priority over my other PRs! |
@tom93 It works™ |
Yay, thanks! |
Just here for some clarity @tom93 . I am trying to build common separately as offline library. |
Just add the following to Calculator's includeBuild("path/to/the/cloned/Commons/") If both Commons and Calculator are in the same directory, you can just do:
|
Will try this and get back. |
@sauravpradhan: includeBuild works for me.
Suggestions:
• make sure you are using a clean worktree (except for the `includeBuild` line, which should be the only change); in particular, make sure you undo the changes you made to build.kts, and if you added any .aar files remove them
• build from the command line using `./gradlew assembleDebug` instead of from Android Studio
(I tested using commit 46840f11 of Calculator and commit 17b79e581 of Commons.)
In the past I also managed to get it working using the Maven local repository. It's more hassle than includeBuild, but the build should be somewhat faster and it avoids some strange compilation errors that occur in some projects (the Calculator project doesn't have such errors).
|
@tom93 Thank you for the suggestion. These were throwing errors and declaring own "corner_radius". This should solve all errors as of now. |
Checklist
Affected Android/Custom ROM version
N/A
Affected device model
N/A
How did you install the apps?
None
Which apps are affected?
No response
Steps to reproduce the bug
When I try to build one of the apps against a local version of Commons (using Maven's local repository) I run into errors. I think it's due to an issue with the configuration of the MavenPublication task, but maybe I'm just doing something wrong -- how do you normally build the apps against a locally-modified version of Commons?
Here are the steps I've been using:
./gradlew publishToMavenLocal
commons = "..."
line to the same version. Also edit settings.gradle.kts and insertmavenLocal()
at the top of the dependencyResolutionManagement section../gradlew assembleDebug
Expected behavior
It should succeed.
Actual behavior
It fails with:
If I add --debug then I get:
Screenshots/Screen recordings
No response
Additional information
When I look in ~/.m2/repository, I the .pom file is actually in ~/.m2/repository/org/fossify/commons/release/1.0.0-local/release-1.0.0-local.pom and has the following contents:
For comparison, https://jitpack.io/org/fossify/commons/b34aef8104/commons-b34aef8104.pom has the following contents (note that different groupId and artifactId):
This makes me think that the Maven configuration is incorrect. The following patch fixes the problem for me (but I have no idea if it will cause problems with jitpack):
I had to change how the namespace is set in commons/build.gradle.kts because it is no longer equal to the groupId. I'm not sure what the best practice is with respect to hard-coding (like I did for the namespace) or using a string from gradle/libs.versions.toml (like in MavenPublication); personally I think gradle/libs.versions.toml somewhat overkill and not really the right place for these non-dependency constants.
The text was updated successfully, but these errors were encountered: