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

CI: Use setup-msys2 options 'location' and 'pacboy' #2763

Closed
wants to merge 1 commit into from

Conversation

umarcor
Copy link
Contributor

@umarcor umarcor commented Nov 4, 2021

Reliance on the 'prefix' broke usage of cocotb with setup-msys2 v2.5.0, because the default installation location was changed.
As a workaround, setup-msys2 was pinned to v2.4.2 in this repo.
In release v2.6.0 of setup-msys2, two options were added:

  • location: allows to specify the location of the MSYS2 installation.
  • pacboy: allows simplifying the syntax for installing MINGW packages.

Having the Action pinned, prevents usage of pacboy. However, using option 'location' allows unpinning the Action, so that 'pacboy' can be used.

This PR uses both 'location' and 'pacboy'.

Refs:

@ktbarrett
Copy link
Member

Not sure I understand why "location" is necessary. This seems to be an issue with setup-msys2 and not a user issue?

@imphil
Copy link
Member

imphil commented Nov 4, 2021

It's most likely the workaround for msys2/setup-msys2#167? If so, @umarcor please add a comment to this line in the config file.

@umarcor
Copy link
Contributor Author

umarcor commented Nov 4, 2021

Not sure I understand why "location" is necessary. This seems to be an issue with setup-msys2 and not a user issue?

  • cocotb relies on the "hardcoded" prefix in the Python installation.
  • Until v2.4.2, setup-msys2 used one location.
    • setup-msys2 is used by MSYS2 CI for building packages.
    • Python has the hardcoded prefix used when the package is built in MSYS2's CI.
  • In v2.5.0, the default location used by setup-msys2 was changed.
    • However, the Python packages were not rebuilt yet.
    • Therefore, packages do still have the "old" prefix, but using a new version of setup-msys2 will install them in the "new" location.
    • That crashes cocotb's installation, and it was pinned to v2.4.2 to avoid it.
  • In v2.6.0, feature "pacboy" was introduced, which simplifies installing packages for multiple environments (as it reads the package name prefix from an envvar instead of requiring it explicitly).
    • I want to enhance the workflow using pacboy, but I cannot because the Action is pinned.
    • Unpinning the Action is possible if option "location" is used for preserving the "old" prefix.

Using the configuration in this PR, everything will be green until Python packages in MSYS2 are rebuilt and use the "new" prefix. Then, we can remove location here and close #2739.

Alternatively, the issue might be silently fixed, if msys2-contrib/cpython-mingw/issues/51 is solved. I'm subscribed to that issue.

@umarcor please add a comment to this line in the config file.

Done.

@ktbarrett
Copy link
Member

ktbarrett commented Nov 6, 2021

cocotb relies on the "hardcoded" prefix in the Python installation.

Doesn't this mean that every user is also broken since 'prefix' config var won't be correct? After reading the linked issue, I'm fairly confident that msys2's Python installation is misconfigured. I'm fine with bringing in a hack, as long as it's forwards compatible with the proper solution, but it's moot if the end user is broken.

@umarcor
Copy link
Contributor Author

umarcor commented Nov 6, 2021

Doesn't this mean that every user is also broken since 'prefix' config var won't be correct? After reading the linked issue, I'm fairly confident that msys2's Python installation is misconfigured.

It is correct that this is a misconfiguration in MSYS2's Python (see msys2-contrib/cpython-mingw#51). However, not every other user is also broken. For instance, the following CI's are using latest setup-msys2 and Python without problems:

@ktbarrett
Copy link
Member

ktbarrett commented Nov 7, 2021

Looks like users are also broken by this, #2740 (comment). The pacboy changes look good, but the location changes are going to need to be fixed upstream, so I think this should be put on hold until we have a fix. We should be testing as users will have their environment configured.

@umarcor
Copy link
Contributor Author

umarcor commented Nov 8, 2021

The pacboy changes look good, but the location changes are going to need to be fixed upstream, so I think this should be put on hold until we have a fix.

Since pacboy cannot be used without bumping the Action, I'll convert this to a draft.

We should be testing as users will have their environment configured.

Users are unlikely to have their environment configured as in CI. MSYS2's autobuild and the default setup-msys2 do not use C:\msys64 (which is the expected local location that most users will use).

You might want to use release: false (https://github.com/msys2/setup-msys2#release). That will use the installation provided by GitHub, instead of a clean one. However on environments other than windows-2022, that installation includes several packages than can delay the update procedure. Hence, should you do so, I recommend changing runs-on: windows-latest to runs-on: windows-2022.

Reliance on the 'prefix' broke usage of cocotb with setup-msys2 v2.5.0.
As a workaround, it was pinned to v2.4.2.
In release v2.6.0, two options were added to setup-msys2:
* location: allows to specify the location of the MSYS2 installation.
* pacboy: allows simplifying the syntax for installing MINGW packages.
Using option 'location' allows unpinning the Action, so that 'pacboy'
can be used.
@marlonjames
Copy link
Contributor

#2964 removed mingw tests from CI

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.

None yet

4 participants