Skip to content

Conversation

@AbdeMohlbi
Copy link
Contributor

@AbdeMohlbi AbdeMohlbi commented Nov 1, 2024

updated mostRecentSemanticVersion func to handle strings like "8.6-rc-2" .
see :

/**
* Compares semantic versions ignoring labels.
*
* If the versions are equal (ignoring labels), returns one of the two strings arbitrarily.
*
* If minor or patch are omitted (non-conformant to semantic versioning), they are considered zero.
* If the provided versions in both are equal, the longest version string is returned.
* For example, "2.8.0" vs "2.8" will always consider "2.8.0" to be the most recent version.
* TODO: Remove this or compareVersionStrings. This does not handle strings like "8.6-rc-2".
*/

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 1, 2024
@AbdeMohlbi
Copy link
Contributor Author

i have no idea how to write tests for this one

@reidbaker
Copy link
Contributor

@gmackall I think this is not unit testable given our current structure. Can you confirm if that is true?

@AbdeMohlbi
Copy link
Contributor Author

Something i noticed is that this function does compare the ndk versions of two plugins and AFAIK there is no ndk version that have letters in it , i'm i right ?

@AbdeMohlbi
Copy link
Contributor Author

this seems related :

// Helper class to parse the versions that are provided as plain strings (Gradle, Kotlin) and
// perform easy comparisons. All versions will have a major, minor, and patch value. These values
// default to 0 when they are not provided or are otherwise unparseable.
// For example the version strings "8.2", "8.2.2hfd", and "8.2.0" would parse to the same version.
class Version(val major: Int, val minor: Int, val patch: Int) : Comparable<Version> {
companion object {
fun fromString(version: String): Version {
val asList: List<String> = version.split(".")
val convertedToNumbers: List<Int> = asList.map { it.toIntOrNull() ?: 0 }
return Version(
major = convertedToNumbers.getOrElse(0, { 0 }),
minor = convertedToNumbers.getOrElse(1, { 0 }),
patch = convertedToNumbers.getOrElse(2, { 0 })
)
}
}
override fun compareTo(other: Version): Int {
if (major != other.major) {
return major - other.major
}
if (minor != other.minor) {
return minor - other.minor
}
if (patch != other.patch) {
return patch - other.patch
}
return 0
}
override fun toString(): String {
return major.toString() + "." + minor.toString() + "." + patch.toString()
}
}

@AbdeMohlbi
Copy link
Contributor Author

AbdeMohlbi commented Nov 17, 2024

using groovy online compiler, u can test the code :

 static String mostRecentSemanticVersion(String version1, String version2) {
        def v1Parts = version1.tokenize('.-')
        def v2Parts = version2.tokenize('.-')

        for (int i = 0; i < Math.max(v1Parts.size(), v2Parts.size()); i++) {
            def v1Part = i < v1Parts.size() ? v1Parts[i] : '0'
            def v2Part = i < v2Parts.size() ? v2Parts[i] : '0'

            def v1Num = v1Part.isNumber() ? v1Part.toInteger() : 0
            def v2Num = v2Part.isNumber() ? v2Part.toInteger() : 0

            if (v1Num != v2Num) {
                return v1Num > v2Num ? version1 : version2
            }

            if (v1Part.isNumber() && !v2Part.isNumber()) {
                return version1
            } else if (!v1Part.isNumber() && v2Part.isNumber()) {
                return version2
            } else if (v1Part != v2Part) {
                return comparePreReleaseIdentifiers(v1Part, v2Part) ? version1 : version2
            }
        }

        // If versions are equal, return the longest version string
        return version1.length() >= version2.length() ? version1 : version2
    }

    static boolean comparePreReleaseIdentifiers(String v1Part, String v2Part) {
        def v1PreRelease = v1Part.replaceAll("\\D", "")
        def v2PreRelease = v2Part.replaceAll("\\D", "")

        return v1PreRelease < v2PreRelease
    }
    
        println mostRecentSemanticVersion("1.2", "1.2.0")           // Output: 1.2.0
        println mostRecentSemanticVersion("1.0", "1")               // Output: 1.0
        println mostRecentSemanticVersion("1.2.0-alpha", "1.2")     // Output: 1.2
        println mostRecentSemanticVersion("1.2.3", "1.2.3")         // Output: 1.2.3
        println mostRecentSemanticVersion("1.2.3-beta", "1.2.3")    // Output: 1.2.3
        println mostRecentSemanticVersion("1.2.3", "1.2.3.4")       // Output: 1.2.3.4
        println mostRecentSemanticVersion("rc-2", "rc-1")           // Output: rc-2
        println mostRecentSemanticVersion("8.7-rc-1", "8.7")        // Output: 8.7
        println mostRecentSemanticVersion("8.7-rc-1", "8.7.2")      // Output: 8.7.2
        println mostRecentSemanticVersion("8.7.2", "8.7.1")         // Output: 8.7.2  

@AbdeMohlbi AbdeMohlbi marked this pull request as ready for review November 17, 2024 17:39
@reidbaker reidbaker marked this pull request as draft November 19, 2024 19:33
@reidbaker
Copy link
Contributor

Marking as draft since pr description says WIP

@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@AbdeMohlbi
Copy link
Contributor Author

AbdeMohlbi commented Nov 19, 2024

Marking as draft since pr description says WIP

fair, i forget to change it .
although I still don't know if is it working right ?

@reidbaker reidbaker requested a review from jesswrd December 17, 2024 16:40
@AbdeMohlbi AbdeMohlbi marked this pull request as ready for review January 7, 2025 09:54
@reidbaker
Copy link
Contributor

@AbdeMohlbi getting this on our review queue. @jesswrd will take a first pass.

int num2 = version2Tokenized[i].toInteger()
if (num1 > num2) {
return version1
static String mostRecentSemanticVersion(String version1, String version2) {
Copy link
Member

Choose a reason for hiding this comment

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

We should either define the input space to be only Major.Minor.Patch in the above doc, just to be clear that we don't care about cases like 8.7-rc-2 vs 8.7.0.1, or otherwise handle them in way that compares initially based on a tokenization of . (i.e., here 7-rc-2 in the former gets compares to 7 in the latter)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for the late response i have some issues with github , so i'm still a bit confused now , shoud it do something like that now ?

version1 version2 Result
1.2.3 1.2.4 1.2.4
1.2.3-rc-1 1.2.3 1.2.3
1.2.3-beta 1.2.3-alpha 1.2.3
1.2.3.1 1.2.3 1.2.3
8.7-rc-2 8.7.0.1 8.7.0

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply - I made that comment during a triage meeting, and on second reading the comparison method seems good to me

Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! LGTM

@gmackall
Copy link
Member

You may need to update the branch, as it looks like the ci.yaml check failed here for reasons I presume are unrelated (and that check in particular can't be re-run from the github ui without updating the branch).

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants