Skip to content

[Bug] Confusing version comparison: version2 = version1.EndsWith("+") #442

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

Closed
qbit86 opened this issue Jun 22, 2021 · 3 comments · Fixed by #443
Closed

[Bug] Confusing version comparison: version2 = version1.EndsWith("+") #442

qbit86 opened this issue Jun 22, 2021 · 3 comments · Fixed by #443
Assignees
Milestone

Comments

@qbit86
Copy link

qbit86 commented Jun 22, 2021

[REQUIRED] Please fill in the following fields:

  • Unity editor version: 2018.4.10f1
  • External Dependency Manager version: 1.2.165
  • Source you installed EDM4U: .unitypackage
  • Features in External Dependency Manager in use: Android Resolver, VersionHandler
  • Plugins SDK in use: Firebase, Facebook, etc.
  • Platform you are using the Unity editor on: Mac, Windows

[REQUIRED] Please describe the issue here:

I was reading sources of Dependency class investigating another issue. And was confused by this argument coercion in IsGreater() method [1]:

version2 = version1.EndsWith("+") ?
    version2.Substring(0, version2.Length - 1) : version2;

Shouldn't it be version2?

version2 = version2.EndsWith("+") ? ...

Or with TrimEnd() [2]:

version2 = version2?.TrimEnd(...);

Please answer the following, if applicable:

What's the issue repro rate?

100%

How can we make the problem occur?

Take a look at Dependency.IsGreater(string version1, string version2)

[1] source/JarResolverLib/src/Google.JarResolver/Dependency.cs#L196
[2] https://docs.microsoft.com/en-us/dotnet/api/system.string.trimend?view=net-5.0

@qbit86 qbit86 added new to be triaged type: question labels Jun 22, 2021
@google-oss-bot
Copy link

This issue does not seem to follow the issue template. Make sure you provide all the required information.

@chkuang-g
Copy link
Collaborator

chkuang-g commented Jun 22, 2021

Thanks for catching this!

This should be fixed in #443 and will be included in the next release.

If you need the patch immediately, you can build EDM4U from source using:

# Build .unitypackage to build/ folder
./gradlew buildPlugin

# Build .tgz to build/ folder
./gradlew buildUpmPlugin

@chkuang-g chkuang-g added this to the 1.2.166 milestone Jun 22, 2021
@DellaBitta
Copy link
Collaborator

I'm going to close this issue for now. If the upcoming change does not fix your issue in the next release, then please re-open this ticket to let us know! Thanks!

@DellaBitta DellaBitta self-assigned this Jun 28, 2021
chkuang-g added a commit that referenced this issue Jun 30, 2021
- Version 1.2.166 - Jun 30, 2021
* All - Fixed #440 and fixed #447 by specifying the parameter type while calling
  `GetApplicationIdentifier()` Unity API using reflection, due to a new
  overloaded method introduced in Unity 2021.2.
* Android Resolver - Fixed #442 by patching `Dependency.IsGreater()` when the
  version strings end '+'.

Change-Id: I45b335d7c196d6b4d03e940b3b0f87f2767a5495
@googlesamples googlesamples locked and limited conversation to collaborators Jul 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants