Skip to content

Conversation

minhqdao
Copy link
Contributor

@minhqdao minhqdao commented Aug 1, 2024

  • Remove msystem: msys from CI.
  • Install fypp via msys2 package instead of pip.

Address #853.

@minhqdao minhqdao requested a review from jvdp1 August 2, 2024 10:03
@zoziha
Copy link
Contributor

zoziha commented Aug 4, 2024

@minhqdao, thank you for your sharing.

As far as I know, msystem:MSYS does not have the fypp package. In this PR, msystem:MSYS will actually use the tools of the msystem:MINGW64 environment, that is to say, msystem:MSYS has no actual effect for CI testing. Combined with the topic of #853, msystem:MSYS should be removed.

I have been a user of MSYS2 for few years. I believe that msystem:MSYS was originally only used to provide an initial Unix-like compilation environment and tools for other environments, and almost no one will use it as a development environment.


It is sufficient for Stdlib to use msystem:MINGW64, msystem:UCRT64, and msystem:CLANG64. MSYS2 also plans to abandon the 32-bit environment, so we should remove the tests of msystem:MSYS and the 32-bit environment (maybe later); Since Flang and LFortran in the msystem:CLANG64 environment are not ready yet, CLANG64 does not need to be supported at present.

@minhqdao
Copy link
Contributor Author

minhqdao commented Aug 4, 2024

I see, it was probably not the best idea to merge the two setups then – I can revert that. In fact, the idea for this PR was only to fix the setup for msystem: mingw64. So we can still do that and leave msystem: msys as is for now because it is still working. If there's a consensus on removing msys entirely, I'm happy to do that in a separate PR.

@minhqdao
Copy link
Contributor Author

minhqdao commented Aug 4, 2024

Ok, I see it was msystem: msys which failed. Let's probably remove it then.

@minhqdao minhqdao changed the title Install fypp via msys2 package Remove msys from CI and install fypp via msys2 package Aug 4, 2024
@minhqdao minhqdao requested a review from zoziha August 4, 2024 10:37
Copy link
Contributor

@zoziha zoziha left a comment

Choose a reason for hiding this comment

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

@minhqdao, thanks, LGTM.

@minhqdao
Copy link
Contributor Author

Will merge this so the CI stops failing. If there's ever the need to bring back msystem: msys, we can do #853 (comment).

@minhqdao minhqdao merged commit 1d27d27 into master Aug 21, 2024
@minhqdao minhqdao deleted the install-fypp-differently branch August 21, 2024 19:15
@chuckyvt chuckyvt mentioned this pull request Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants