fortran-lang / fpm

Fortran Package Manager (fpm)
https://fpm.fortran-lang.org
MIT License
870 stars 97 forks source link

OS detection is faulty #242

Open urbanjost opened 3 years ago

urbanjost commented 3 years ago

Running in a CygWin environment I had to unset the environment variable OS which was set for the MSWIndows applications and inherited by the CygWin application in order for the routine testing for system type to identify the environment as a CygWin/POSIX environment instead of a default MSWindows environment. Looks like the routine automatically assumes if the OS variable is set it is MSWindows. At least need a note to that effect somewhere; but might want to change the test. Specifically for CygWin there are several environment variables and the /usr/bin/cyg* commands that can be tested for without having to call platform-specific C routines if anyone wants a list. The routine identifying the platform is slated for replacement by a stdlib routine I believe, but that might be a ways off.

LKedward commented 3 years ago

The Windows check (using 'OS'): https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L36-L39

occurs before the cygwin check (using 'OSTYPE'): https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L64-L67

but since cygwin imports all Windows environment variables, the Windows check will take precedence over the cygwin one and always return OS_WINDOWS.

A possible fix could be to check the environment variable 'OS' after 'OSTYPE' and not before - what do you think @interkosmos?

arjenmarkus commented 3 years ago

I just checked: Cygwin defines OSTYPE (to be "cygwin") whereas plain Windows does not define it - instead it uses OS. So the logic should take that non-existence into account. But checking OSTYPE first seems a reasonable solution.

Op di 17 nov. 2020 om 14:37 schreef Laurence Kedward < notifications@github.com>:

The Windows check (using 'OS'):

https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L36-L39

occurs before the cygwin check (using 'OSTYPE'):

https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L64-L67

but since cygwin imports all Windows environment variables https://cygwin.com/cygwin-ug-net/setup-env.html, the Windows check will take precedence over the cygwin one and always return OS_WINDOWS.

A possible fix could be to check the environment variable 'OS' after 'OSTYPE' and not before - what do you think @interkosmos https://github.com/interkosmos?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fortran-lang/fpm/issues/242#issuecomment-728931518, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN6YR2ALYVCXMPTJ4X7QX3SQJ4AJANCNFSM4TX3JB5A .

arjenmarkus commented 3 years ago

Hm, MinGW poses another complication: if started as mingw64.exe you have a Linux-like environment under Windows that is compatible with plain Windows, whereas msys64.exe takes you closer to Linux. The two environment variables do not allow you to distinguish the two, though I think mingw64.exe is the preferred environment

Op di 17 nov. 2020 om 14:40 schreef Arjen Markus <arjen.markus895@gmail.com

:

I just checked: Cygwin defines OSTYPE (to be "cygwin") whereas plain Windows does not define it - instead it uses OS. So the logic should take that non-existence into account. But checking OSTYPE first seems a reasonable solution.

Op di 17 nov. 2020 om 14:37 schreef Laurence Kedward < notifications@github.com>:

The Windows check (using 'OS'):

https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L36-L39

occurs before the cygwin check (using 'OSTYPE'):

https://github.com/fortran-lang/fpm/blob/c68cf2fbdb40c33636bd50b6a729490ae9d61654/fpm/src/fpm_environment.f90#L64-L67

but since cygwin imports all Windows environment variables https://cygwin.com/cygwin-ug-net/setup-env.html, the Windows check will take precedence over the cygwin one and always return OS_WINDOWS.

A possible fix could be to check the environment variable 'OS' after 'OSTYPE' and not before - what do you think @interkosmos https://github.com/interkosmos?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fortran-lang/fpm/issues/242#issuecomment-728931518, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN6YR2ALYVCXMPTJ4X7QX3SQJ4AJANCNFSM4TX3JB5A .

LKedward commented 3 years ago

Yes I agree with you @arjenmarkus. To distinguish between the mingw64.exe shell and the msys64.exe shell there should be an 'MSYSTEM' environment variable set to 'MINGW64' and 'MSYS' for the two different shells respectively.


Aside: as an example why the two shells should be treated differently, execute_command_line will call the msys bash shell in msys64.exe but call the Windows cmd shell in mingw64.exe. Hence msys should be treated like Linux and mingw64 like Windows.

LKedward commented 3 years ago

I had a look at running fpm under MSYS2 and realised there's another problem with the OS detection routine. The routine looks for the OSTYPE environment variable, however OSTYPE is a bash variable and not a common environment variable. Hence the current implementation is actually unable to identify OS_WINDOWS, OS_CYGWIN, or OS_SOLARIS.

urbanjost commented 3 years ago

Yes, noticed that too but you can do an "export OSTYPE" and "unset OS" as a work-around in your .bashrc as a work-around for Cygwin. If fpm is to support some form of pre-processing OSTYPE in one form or another would be a useful one to have set as well. Not sure if there is a stdlib routine to do this in the works. perhaps a work-around is to assume fpm is compiled with GNU and to add a C routine that looks for __gnu_linux and linux__ and similar variables instead of re-creating something for now. A lot more predefined variables exist for gcc than gfortran for some reason. The main reason I think the code is looking for what OS it is it to determine a file separator character, which I think I have a routine for that just uses INQUIRE and a few other things on the pathname returned for ARG0 to figure out the seperator character as another alternative.

cpp -dM /dev/null
LKedward commented 3 years ago

I agree it would be good for fpm to define some preprocessor variables such as OSTYPE, it's frustrating that many useful variables defined in cpp are excluded from gfortran for no good reason.

However, for fpm we need to detect OSTYPE at runtime due to complications with the various possible Windows systems; in particular, 'plain Windows' binaries (currently supported) can run on msys2/cygwin but run into issues with our filesystem routines implemented via shell commands.

The main reason I think the code is looking for what OS it is it to determine a file separator character

Yes, also to add the .exe suffix on Windows and to determine which shell command to execute for our filesystem routines (to be replaced).

Actually determining which shell command to run at the moment is the main problem on msys2/cygwin, once these routines are replaced with c calls, we should then only need to distinguish *nix from Windows at runtime.

urbanjost commented 3 years ago

In

https://github.com/urbanjost/M_io

I have two routines (which(3f) and separator(3f)) which might be useful, but I have not tested them in several of those environments. The separator procedure tries to use standard Fortran and an INQUIRE on arg0 to determine if a backslash is being used or a slash as a separator, which(3f) tries to use the PATH environment variable to see if a command is in the current path. Otherwise, maybe bundling a little C program and executing like a system command (and assuming gfortran and GNU cpp) would work well.

sloorush commented 1 year ago

@awvwgk could you please reopen this issue because MINGW still poses an issue as mentioned in https://github.com/fortran-lang/fpm/issues/242#issuecomment-728936642