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

--review-pr should check against same ECs from same toolchain #4683

Open
Flamefire opened this issue Oct 15, 2024 · 0 comments
Open

--review-pr should check against same ECs from same toolchain #4683

Flamefire opened this issue Oct 15, 2024 · 0 comments

Comments

@Flamefire
Copy link
Contributor

Flamefire commented Oct 15, 2024

I tried --review-pr on a PR that introduced a binutils-2.42-GCCcore-13.2.0.eb.
That finds binutils-2.42 in other toolchains but not binutils-2.40-GCCcore-13.2.0.eb in the same toolchain.

I suggest to enhance that to also compare against ECs in the same toolchain generation.
This might be useful in general and to catch mistakes like accidentally adding an EC next to an existing one instead of using that.

From the documentation of find_related_easyconfigs:

    The following criteria are considered (in this order) next to common software version criterion, i.e.
    exact version match, a major/minor version match, a major version match, or no version match (in that order).

    (i)   matching versionsuffix and toolchain name/version
    (ii)  matching versionsuffix and toolchain name (any toolchain version)
    (iii) matching versionsuffix (any toolchain name/version)
    (iv)  matching toolchain name/version (any versionsuffix)
    (v)   matching toolchain name (any versionsuffix, toolchain version)
    (vi)  no extra requirements (any versionsuffix, toolchain name/version)

That doesn't exactly describe the "next to common software version criterion", i.e. how that relates to the toolchain list.

What I expected:

    # 1. Same toolchain & version
    # 2. Same toolchain, different version
    # 3. Any toolchain
    # For each:
    #   exact version, major/minor version, major version, any version

But that doesn't seem to be the case.

I suggest to

  • Clarify in the description how the software version matching is related to toolchain matching.
  • Additionally always include for the same toolchain name/version the first non-empty list that matches one of these 8 possibilities: versionsuffix + exact/majmin/maj/any version, any versionsuffix + exact/majmin/maj/anyversion
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

No branches or pull requests

1 participant