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

Android manifest update #4977

Merged
merged 5 commits into from
Oct 17, 2024
Merged

Android manifest update #4977

merged 5 commits into from
Oct 17, 2024

Conversation

Hanmac
Copy link
Contributor

@Hanmac Hanmac commented Apr 7, 2024

this moves the Android Manifest to the more Default Location for the plugins

and enables manifest-merger goal.

This sets the VersionName and generates the VersionCode from it

Agetian
Agetian previously approved these changes Apr 7, 2024
@Hanmac
Copy link
Contributor Author

Hanmac commented Apr 8, 2024

@Agetian maybe you know why on github we don't need to mess with the d8 file?

@Agetian
Copy link
Contributor

Agetian commented Apr 8, 2024

@Agetian maybe you know why on github we don't need to mess with the d8 file?

Hmm, not sure tbh, I haven't investigated the Github way of doing things as far as building and deploying an Android snapshot goes ^^;

@Hanmac
Copy link
Contributor Author

Hanmac commented Apr 26, 2024

@Agetian can you look to replace

Forge.CURRENT_VERSION in there:

public static final String CURRENT_VERSION = "1.6.63-SNAPSHOT";

BuildInfo.getVersionString ?

public static final String getVersionString() {

that is one of the last pieces where the Version needs to be updated

@Agetian
Copy link
Contributor

Agetian commented Apr 26, 2024

Hmm, getVersionString simply returns "GIT" for me (on a snapshot version), with the entire BuildInfo.class.getPackage() method returning "null"... Not sure if it would return a proper version string if executed on an actual release. At any rate, kinda confusing for snapshots as such. I wonder if there's a better way to track the number, hmm...

@Hanmac
Copy link
Contributor Author

Hanmac commented Apr 26, 2024

Hmm, getVersionString simply returns "GIT" for me (on a snapshot version), with the entire BuildInfo.class.getPackage() method returning "null"... Not sure if it would return a proper version string if executed on an actual release. At any rate, kinda confusing for snapshots as such. I wonder if there's a better way to track the number, hmm...

Normally, maven should have put the version inside these Manifest files so it can be accessed by getPackage()
I need to check what is missing

@Hanmac
Copy link
Contributor Author

Hanmac commented Apr 26, 2024

I think we need https://maven.apache.org/shared/maven-archiver/examples/manifest.html

     <configuration>
         <archive>
             <manifest>
                 <addDefaultImplementationEntries>true</addDefaultImplementationEntries>
             </manifest>
         </archive>
     </configuration>

i will check this out at weekend

@Agetian
Copy link
Contributor

Agetian commented Apr 26, 2024

Sounds good!

@Hanmac
Copy link
Contributor Author

Hanmac commented Apr 27, 2024

@Agetian the addDefaultImplementationEntries should already be enabled by default

when i run ./forge.sh i already got the correct Forge Version

As for the Android thing, i tried:

JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64/ mvn -U -B -P android-debug install -Dsign.keystore=forge.keystore -Dsign.alias=Forge -Dandroid.sdk.path=/opt/android-sdk/ -T 1C -DskipTests=true

and i get:

[ERROR] Failed to execute goal com.simpligility.maven.plugins:android-maven-plugin:4.6.1:manifest-merger (update-manifest) on project forge-gui-android: Execution update-manifest of goal com.simpligility.maven.plugins:android-maven-plugin:4.6.1:manifest-merger failed: FeatureName must follow [a-zA-Z0-9][a-zA-Z0-9_]* regex, found -> [Help 1]

but i can't find where the problem is happening (it is difficult to debug inside the maven plugin)

Copy link

This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed

@Hanmac Hanmac added keep no stale and removed no-pr-activity labels Jun 12, 2024
@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 12, 2024

Linked Issue that is causing the problem:
simpligility/android-maven-plugin#814

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 10, 2024

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 10, 2024

@kevlahnota should i try to apply this Commit into this MR too? ad81b0e

@kevlahnota
Copy link
Contributor

@kevlahnota should i try to apply this Commit into this MR too? ad81b0e

Hmm the code there will just remove all non digit from revision - 2.0.00-snapshot, to 2000 and append 0 and MMdd where MM for month and dd for the date so the versioncode if date today is october 10, it will be 200001010 as version code (also append MMdd to the finalname), I don't know if it will work on your MR

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 10, 2024

@kevlahnota i don't understand the config there yet: https://github.com/Card-Forge/android-maven-plugin/blob/b36d05b798f3b5cf1362795f008171309183678e/src/main/java/com/simpligility/maven/plugins/android/standalonemojos/ManifestMergerMojo.java#L120

right now it does reset the VersionCode to 1 🤔

@kevlahnota like i said, right now the manifestVersionCodeUpdateFromVersion doesn't seem to work for me, and the VersionCode in the Manifest is always set to "1".

But i don't understand yet why it does happen, and have no idea on how to debug it

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 10, 2024

@kevlahnota this code right there should have updated the VersionCode

https://github.com/Card-Forge/android-maven-plugin/blob/41bbf85159145c51cb922dde9618c9cf6f138286/src/main/java/com/simpligility/maven/plugins/android/standalonemojos/ManifestMergerMojo.java#L312-L317

in theory, it should have turned 1.6.65-SNAPSHOT into 1006065

but for some reason it doesn't, and I don't know how to debug the maven plugin

not really for snapshots, but that should have been good enough?

@kevlahnota
Copy link
Contributor

@kevlahnota this code right there should have updated the VersionCode

https://github.com/Card-Forge/android-maven-plugin/blob/41bbf85159145c51cb922dde9618c9cf6f138286/src/main/java/com/simpligility/maven/plugins/android/standalonemojos/ManifestMergerMojo.java#L312-L317

in theory, it should have turned 1.6.65-SNAPSHOT into 1006065

but for some reason it doesn't, and I don't know how to debug the maven plugin

not really for snapshots, but that should have been good enough?

did you try to pass the alpha-version?

<configuration>
    <manifestVersionName>${alpha-version}</manifestVersionName>
    <manifestVersionCodeUpdateFromVersion>true</manifestVersionCodeUpdateFromVersion>
</configuration>

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 10, 2024

@kevlahnota this code right there should have updated the VersionCode
https://github.com/Card-Forge/android-maven-plugin/blob/41bbf85159145c51cb922dde9618c9cf6f138286/src/main/java/com/simpligility/maven/plugins/android/standalonemojos/ManifestMergerMojo.java#L312-L317
in theory, it should have turned 1.6.65-SNAPSHOT into 1006065
but for some reason it doesn't, and I don't know how to debug the maven plugin
not really for snapshots, but that should have been good enough?

did you try to pass the alpha-version?

<configuration>
    <manifestVersionName>${alpha-version}</manifestVersionName>
    <manifestVersionCodeUpdateFromVersion>true</manifestVersionCodeUpdateFromVersion>
</configuration>

The default should be ${project.version}, i will test ${alpha-version} as soon as i get home

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 15, 2024

Ugh, looks like I need to rebase it at home

@Hanmac Hanmac marked this pull request as ready for review October 16, 2024 11:17
@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 16, 2024

@kevlahnota on my System, i had problems with the working directory.

It only works correct when calling it in forge-gui-android, otherwise the parameter gets messed up?

@kevlahnota
Copy link
Contributor

kevlahnota commented Oct 16, 2024

@kevlahnota on my System, i had problems with the working directory.

It only works correct when calling it in forge-gui-android, otherwise the parameter gets messed up?

I can't figure it out on my system too since its always missing 4 digits that's why I resort to use the build-helper-maven-plugin (timestamp-property for appending monthdate code, regex-property to retain only numeric characters from revision, still missing 1 digit character so I append 0 before the monthdate code ${snapshot-versionCode}0${month.date} on my PR)

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 16, 2024

missing 4 digits

That's why I added this:
<android.manifestMerger.versionDigits>2,2,2,4</android.manifestMerger.versionDigits>

With that, even 1.2.3 is turned into 102030000, so you can use the last 4 digits for snapshot stuff

@tehdiplomat tehdiplomat merged commit f5b30a6 into master Oct 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants