Skip to content

Conversation

@benoitdion
Copy link
Contributor

@benoitdion benoitdion commented Jun 12, 2020

Use action input androidVersionCode when provided. Generate the androidVersionCode from the version otherwise.

@codecov
Copy link

codecov bot commented Jun 12, 2020

Codecov Report

Merging #101 into master will increase coverage by 0.34%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
+ Coverage   94.69%   95.03%   +0.34%     
==========================================
  Files          14       15       +1     
  Lines         245      262      +17     
  Branches       47       50       +3     
==========================================
+ Hits          232      249      +17     
  Misses         13       13              
Impacted Files Coverage Δ
src/model/docker.js 63.63% <ø> (ø)
src/model/android-versioning.js 100.00% <100.00%> (ø)
src/model/build-parameters.js 100.00% <100.00%> (ø)
src/model/input.js 100.00% <100.00%> (ø)

@webbertakken
Copy link
Member

Note that this warning is caused by the latest versioning update. This is something I will fix (when I have time)

The problem is that it should use this.branch instead of origin/${this.branch} in the versioning script. But this will need to be tested for push pull request pull request from downstream and tag.

##[warning]describe origin/android-versioncode
fatal: Not a valid object name origin/android-versioncode

@benoitdion benoitdion force-pushed the android-versioncode branch from adbe076 to 047f674 Compare June 12, 2020 21:34
@benoitdion
Copy link
Contributor Author

@webbertakken I was just looking into that and added some logging. I might need to find a fix or workaround to continue testing.

@webbertakken
Copy link
Member

webbertakken commented Jun 12, 2020

You might just remove the origin/ part right below where you added the log, or better even replace it with the result of git remote show. That functionality was very tedious to make due to how the official actions/checkout works. Note that there are multiple occurences where it hardcodes origin

@benoitdion benoitdion force-pushed the android-versioncode branch from 16acbab to 5323b0a Compare June 12, 2020 23:57
@benoitdion
Copy link
Contributor Author

@webbertakken

(ignore some of the workarounds to get the build going)

the android build is now failing because the gradle daemon is crashing. This usually means there's an OOM. I'm not seeing this on my builds with a very similar configuration. The main difference is we're running 2019.3.13f1. I see there was an attempt to get 2019.3 builds going but was abandoned due to a license issue. Do you know if it's something we could resume? If so, could you help generate a new license file?

@webbertakken
Copy link
Member

Hi @benoitdion,

Can we somehow prove that this is an out of memory issue?

For version 2019.3 builds work fine with the exception of a few specific versions that have the Unity Docker bug (nothing we can do here except picking one of the working versions). In these same issues some versions that should work for sure have been mentioned.

I know of other builds failing because of an out of disk space issue and people have successfully utilised workarounds for this, like cleaning up the runner. As a more scalable option we're also working to delegate the build process to kubernetes, however this is still a work in progress for now.

I cannot not recall of any attempts where there are out of memory issues or that have been abandoned due to licensing (most people just pick a proven version, also for 2019.3), could you link the issue you're referring to?

Generating a license file should be straight forward, see https://unity-ci.com/docs/github/activation or https://github.com/webbertakken/unity-request-manual-activation-file for this. Or did you mean to have unity builder main repo update to a 2019.3 version? This is something I can do if it is indeed necessary.

@benoitdion
Copy link
Contributor Author

I meant the main repo (see https://github.com/webbertakken/unity-builder/blob/master/.github/workflows/main.yml#L37. We could upgrade all apps, or potentially only android as part of a matrix build. This could also be temporary if we're able to find a better solution to the gradle issues.

Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed). In my experience, this usually means a gradle bug or the process ran out of memory. Usually the latter.

@webbertakken
Copy link
Member

So if I understand correctly, you think the Gradle issue is due to the Unity version?

I'll update the version, but currently I don't understand why you're asking me to do this.

@benoitdion
Copy link
Contributor Author

To be honest, I don't completely understand the issue yet.

I know the gradle errors we're seeing are usually due to out of memory.
A similar set up for my own builds works fine with a 2019.3 version of unity.

It's not clear if upgrading will solve the issue but it feels reasonable to try before digging further.

@webbertakken
Copy link
Member

Progress can be tracked in #102

@webbertakken
Copy link
Member

@benoitdion it's merged

@benoitdion benoitdion force-pushed the android-versioncode branch 2 times, most recently from c813123 to 63abc77 Compare June 15, 2020 20:08
@benoitdion benoitdion mentioned this pull request Jun 15, 2020
@benoitdion benoitdion force-pushed the android-versioncode branch from 9caaf70 to 38ba81e Compare June 16, 2020 14:44
@benoitdion benoitdion changed the title [DRAFT] Add ability to set android versionCode Add ability to set android versionCode Jun 19, 2020
@benoitdion benoitdion changed the title Add ability to set android versionCode Set android versionCode Jun 19, 2020
@benoitdion benoitdion force-pushed the android-versioncode branch from 69a8168 to a4e1dc2 Compare June 19, 2020 01:03
@benoitdion benoitdion marked this pull request as ready for review June 19, 2020 01:15
Copy link
Member

@webbertakken webbertakken left a comment

Choose a reason for hiding this comment

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

Good work man. Looking forward to merging this!

Few comments;

README.md Outdated

Configure the android `versionCode`.

When not specified, the version code is generated from the version using the `major * 1000000 + minor * 10000 + patch` scheme;
Copy link
Member

Choose a reason for hiding this comment

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

probably we need to make it possible to have more than 99 minor versions, since it's may not be very uncommon in some use cases.

}

// The greatest value Google Plays allows is 2100000000.
// Allow for 4 patch digits, 2 minor digits and 3 major digits.
Copy link
Member

Choose a reason for hiding this comment

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

To me 3-3-4 digits would make more sense than 3-2-4 digits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only have room for 9 digits so unfortunately, that won't work. I prefer 3-2-4 over 3-3-3 because if people don't increment major/minor often, it's very convenient to have 4 digits.

I use ${{ github.run_number }} in my builds since it's a bit safer and it's not that important to have versionCode correspond to versionName. Preferences on this vary though.

Copy link
Member

Choose a reason for hiding this comment

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

So if google play allows 2 100 000 000, that looks like 10 digits to me. Could you explain how you derive 9 digits?

My use case would be to have major always at 0 or 1 or 2. Perhaps if the project gets enormously successful i might get up to 20 in several years, but never 99, let alone need more digits than for minor.

We only have room for 9 digits so unfortunately, that won't work. I prefer 3-2-4 over 3-3-3 because if people don't increment major/minor often, it's very convenient to have 4 digits.

Minor however I use when a new feature gets released. I will almost certainly reach 0.99.0 and 1.99.0. As major digit is reserved for semantically major releases I think 2 digits for minor level is not really safe either.

I would say that depending on the answer to the first question either 2-3-4 or 3-3-4 would be the best compromise. Let me know what you think.

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'd really like to treat the most significant digit as reserved. When you get to 1000000000, that should be a strong warning sign to consider a different versioning scheme to avoid running out of numbers.

With 3-2-4, I was thinking of apps like Firefox that increment the major monthly. We could also go 3-3-3 and aggressively throw errors when one of the numbers is over 999. In the future, there could also be an option for users to specify how the number is generated.

Copy link
Member

Choose a reason for hiding this comment

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

I see where you're coming from and I think there is some good logic in that. However we're not talking about enterprise software exposing a public API here but about builds for games or graphical programs that are generated by the Unity Editor. Versioning for games to me seems a lot more about having a unique incremental identifier for each build than about anything else.

Don't get me wrong, I'm all for making it work for all use cases, and even for enterprise level scenarios where possible, but like i said, 2 digits for the minor version might realistically already not be enough for my hobby project, let alone for bigger projects. See also this thread where we briefly discussed some scenarios about versioning and digits for the store too. Back then we came up with x-3-3.

I'm not fully aligned with you on reserve the most significant number. 3-3-4 seems to me like a very reasonable option; even when upping the major version every month it will take almost 18 years until you run out of versions. To put things in perspective, by that time app stores may have ceased existing altogether and teams that are enterprise level enough will most likely not use automatic versioning in the first place.

If there's indeed a real use case for games where we need more than 210 major versions, which I highly doubt, then we should instead make the assumption that either a minor version is updated between every 999 commits or that semantic versioning by tag is not used at all until the first major version is upgraded. This assumption would allow for a patch level with 3 digits. I'll leave the decision between 3-3-4 and 3-3-3 up to you, as you likely have more experience with versioning for mobile stores than I.

In the future, there could also be an option for users to specify how the number is generated.

This is already possible in the current release. We could apply custom, tag and other strategies to Android versioning too if you like.

Copy link
Contributor Author

@benoitdion benoitdion Jun 20, 2020

Choose a reason for hiding this comment

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

I'll start us with 3-3-3, 3-3-4 is backward compatible with 3-3-3 if you don't give any semantic meaning to the numbers and we wanted more room in the future.

However we're not talking about enterprise software exposing a public API here but about builds for games or graphical programs that are generated by the Unity Editor. Versioning for games to me seems a lot more about having a unique incremental identifier for each build than about anything else.

I'm with you 100% on that. We don't actually use semver for our apps since there's really no consumer of that version and incrementing a number is a simpler mental model. The configuration we're using is

versioning: 'Custom'
version: 0.0.${{ github.run_number }}
androidVersionCode: ${{ github.run_number }}

}

// The greatest value Google Plays allows is 2100000000.
// Allow for 4 patch digits, 2 minor digits and 3 major digits.
Copy link
Member

Choose a reason for hiding this comment

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

I see where you're coming from and I think there is some good logic in that. However we're not talking about enterprise software exposing a public API here but about builds for games or graphical programs that are generated by the Unity Editor. Versioning for games to me seems a lot more about having a unique incremental identifier for each build than about anything else.

Don't get me wrong, I'm all for making it work for all use cases, and even for enterprise level scenarios where possible, but like i said, 2 digits for the minor version might realistically already not be enough for my hobby project, let alone for bigger projects. See also this thread where we briefly discussed some scenarios about versioning and digits for the store too. Back then we came up with x-3-3.

I'm not fully aligned with you on reserve the most significant number. 3-3-4 seems to me like a very reasonable option; even when upping the major version every month it will take almost 18 years until you run out of versions. To put things in perspective, by that time app stores may have ceased existing altogether and teams that are enterprise level enough will most likely not use automatic versioning in the first place.

If there's indeed a real use case for games where we need more than 210 major versions, which I highly doubt, then we should instead make the assumption that either a minor version is updated between every 999 commits or that semantic versioning by tag is not used at all until the first major version is upgraded. This assumption would allow for a patch level with 3 digits. I'll leave the decision between 3-3-4 and 3-3-3 up to you, as you likely have more experience with versioning for mobile stores than I.

In the future, there could also be an option for users to specify how the number is generated.

This is already possible in the current release. We could apply custom, tag and other strategies to Android versioning too if you like.

Use action input `androidVersionCode` when provided. Generate the androidVersionCode from the version otherwise.
@benoitdion benoitdion force-pushed the android-versioncode branch from 4091a0c to d6e4d4f Compare June 24, 2020 22:02
@benoitdion
Copy link
Contributor Author

@webbertakken cleaned up git history and addressed review comments. Do you mind taking another look?

@webbertakken
Copy link
Member

@benoitdion great work and thanks for addressing the comments!

@webbertakken webbertakken merged commit bdc3a88 into game-ci:master Jun 24, 2020
@benoitdion benoitdion mentioned this pull request Jun 25, 2020
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.

2 participants