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

[easy_train] Potential issue with msys2 and PATH #186

Open
Sopel97 opened this issue May 30, 2022 · 13 comments
Open

[easy_train] Potential issue with msys2 and PATH #186

Sopel97 opened this issue May 30, 2022 · 13 comments

Comments

@Sopel97
Copy link
Member

Sopel97 commented May 30, 2022

...msys2\mingw64\bin needs to be in PATH, but that might not be setup by default, in which case we need to modify the environment programatically

@Sopel97 Sopel97 changed the title Potential issue with msys2 and PATH [easy_train] Potential issue with msys2 and PATH May 30, 2022
@ppigazzini
Copy link

I just installed msys2 in a sandbox with the latest official installer, the path is not updated automatically and there is not an option.
The user can add "C:\msys64\usr\bin;C:\msys64\mingw64\bin;" in the path environment variable (first folder for msys2 applications like grep, second one for the mingw-w64 applications if any).
Passing an env to subprocess is an easy option but the user must be aware to update the msys2 path with a not standard msys2 installation. Chocolatey at example installs msys2 in C:\tools\msys64\

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

we can probably infer the path from where gcc, because we already require gcc in path, and the only way for newer gcc on windows appears to be msys2

@ppigazzini
Copy link

ppigazzini commented May 30, 2022

The standard module winreg is not an option because the msys2 installer (official and with choco) makes a nearly portable install and doesn't register the application under HKEY_LOCAL_MACHINE\Software.
The cmd where or the powershell Get-ChildItem cmdlet seems to be the only options for an automatic discovery, but there is the risk to make a long search in a wrong disk.
A safer alternative is to run gcc -v with subprocess adding to the env a couple of standard paths (eg C:\msys64\mingw64\bin and C:\tools\msys64\mingw64\bin) and on a failure ask the right path to the user.

There are other mingw-w64 recent distributions like https://jmeubank.github.io/tdm-gcc/articles/2021-05/10.3.0-release but IMO better to have msys2 as requirement.

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

According to https://superuser.com/a/480115/388191 where only searches cwd and directories from PATH. So we should be fine.

@ppigazzini
Copy link

ppigazzini commented May 30, 2022

In this case where works only if we ask the user to update the PATH (a viable option of course).

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

Does where gcc not work in the sandbox you set up? If gcc was not in path then the issue reported on discord would have failed during verification, much earlier.

@ppigazzini
Copy link

Check where /? to view all the options. It scanned all C: drive to find grep.exe

C:\Users\WDAGUtilityAccount>where /R C:\ grep.exe
C:\msys64\usr\bin\grep.exe

Another option: we use a worker.cmd launcher on windows, the user doesn't need to update the PATH

@echo off
set PATH=C:\tools\msys64\mingw64\bin;C:\tools\msys64\usr\bin;%PATH%
env\bin\python3.exe -i worker.py

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

Alternatively we may have to fall back to have the user provide msys2 directory as an argument, with an attempt to infer it otherwise.

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

Another option: we use a worker.cmd launcher on windows, the user doesn't need to update the PATH

What do we set the path to though

@ppigazzini
Copy link

Another option: we use a worker.cmd launcher on windows, the user doesn't need to update the PATH

What do we set the path to though

Standard installation paths used by chocolatey, see the scripts
https://github.com/ppigazzini/worker-setup

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

Ah, so you basically want the script to install msys? My msys2 directory is close to 4GB, I don't think it's wise to force a setup.

@ppigazzini
Copy link

ppigazzini commented May 30, 2022

Yes, the windows worker setup is meant for the CPU contributor not able to follow the installation tutorial on the wiki. Chocolatey has the plus to auto update msys2 after the installation and the minus to use a not standard installation path.
The script installs only the required applications (and uses wget + unzip instead than the space hungrier git) , but you are right that msys2 takes some space. Fortunately users never protested about that.

# install packages if not already installed
pacman -S --noconfirm --needed unzip make mingw-w64-x86_64-gcc mingw-w64-x86_64-python3

@Sopel97
Copy link
Member Author

Sopel97 commented May 30, 2022

Okay. I think you have a point and it would be a good idea to fallback to automatic msys installation if not provided by the user. I'll try to prepare something around tomorrow.

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

2 participants