alire-project / alire

Command-line tool from the Alire project and supporting library
GNU General Public License v3.0
277 stars 49 forks source link

Path on windows with slashes and backslashes #1178

Closed pjljvandelaar closed 6 months ago

pjljvandelaar commented 2 years ago

Dear Alire developers,

While trying to find the root cause of a bug that only occurs on windows, I noticed the following: when typing

 alr printenv --powershell

in the tests directory of the Rejuvenation-Ada project, one gets (something like)

$env:ALIRE = "True"
$env:AUNIT_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\aunit_22.0.0_cbd7a80a"
$env:C_INCLUDE_PATH = "C:\Users\laarpjljvd\.cache\alire\msys64\mingw64\include"
$env:GNATCOLL_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_22.0.0_620c2f23"
$env:GNATCOLL_BUILD_MODE = "PROD"
$env:GNATCOLL_GMP_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_gmp_22.0.0_f3732e5d"
$env:GNATCOLL_ICONV_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_iconv_22.0.0_f3732e5d"
$env:GNATCOLL_OS = "windows"
$env:GNATCOLL_VERSION = "22.0.0"
$env:GNAT_NATIVE_ALIRE_PREFIX = "C:\Users\laarpjljvd\.config\alire\cache\dependencies\gnat_native_11.2.4_2f9c5d6d"
$env:GPRBUILD_ALIRE_PREFIX = "C:\Users\laarpjljvd\.config\alire\cache\dependencies\gprbuild_22.0.1_c842bbc5"
$env:GPR_PROJECT_PATH = "C:\Users\laarpjljvd\.config\alire\cache\dependencies\gnat_native_11.2.4_2f9c5d6d;C:\Users\laarpjljvd\.config\alire\cache\dependencies\gprbuild_22.0.1_c842bbc5;C:\path\to\Rejuvenation-Ada;C:\path\to\Rejuvenation-Ada\tests;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\aunit_22.0.0_cbd7a80a\lib/gnat;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_22.0.0_620c2f23;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_gmp_22.0.0_f3732e5d\gmp;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\gnatcoll_iconv_22.0.0_f3732e5d\iconv;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\langkit_support_22.0.0_d43df3a9;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libadalang_22.0.0_5f365aa4;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libgpr_22.0.0_30e39dcc\gpr;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\distrib;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\dom;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\input_sources;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\sax;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\schema;C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27\unicode"
$env:LANGKIT_SUPPORT_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\langkit_support_22.0.0_d43df3a9"
$env:LIBADALANG_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libadalang_22.0.0_5f365aa4"
$env:LIBGMP_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libgmp_6.2.1_system"
$env:LIBGPR_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libgpr_22.0.0_30e39dcc"
$env:LIBICONV_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\libiconv_1.17.0_system"
$env:LIBRARY_PATH = "C:\Users\laarpjljvd\.cache\alire\msys64\mingw64\lib"
$env:PATH = "C:\Users\laarpjljvd\.config\alire\cache\dependencies\gprbuild_22.0.1_c842bbc5/bin;C:\Users\laarpjljvd\.config\alire\cache\dependencies\gnat_native_11.2.4_2f9c5d6d/bin;C:\Users\laarpjljvd\.cache\alire\msys64\usr\bin;C:\Users\laarpjljvd\.cache\alire\msys64\usr\local\bin;C:\Program Files\Common Files\Oracle\Java\javapath;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\Program Files\dotnet\;C:\Program Files\Microsoft SQL Server\130\Tools\Binn\;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn\;C:\WINDOWS\System32\OpenSSH\;C:\Program Files\TortoiseSVN\bin;C:\Program Files\Git\cmd;C:\Program Files (x86)\dotnet\;C:\Program Files\TortoiseGit\bin;%SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;%SYSTEMROOT%\System32\WindowsPowerShell\v1.0\;%SYSTEMROOT%\System32\OpenSSH\;C:\Program Files\Alire\bin;C:\GNATPRO\23.0w\bin;C:\GNATPRO\23.0w-20220211\bin;C:\Users\laarpjljvd\AppData\Roaming\local\bin;C:\Users\laarpjljvd\AppData\Local\Programs\Python\Python39\Scripts\;C:\Users\laarpjljvd\AppData\Local\Programs\Python\Python39\;C:\Users\laarpjljvd\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Java\jdk1.8.0_171\bin;C:\Program Files (x86)\Graphviz2.38\bin;C:\Program Files (x86)\GnuWin32\bin;C:\Users\laarpjljvd\AppData\Local\Programs\MiKTeX 2.9\miktex\bin\x64\;C:\Program Files (x86)\Inno Setup 6;C:\Users\laarpjljvd\.dotnet\tools;C:\Program Files Local\apache-maven-3.6.3\bin;C:\Program Files Local\cvc4;C:\Program Files Local\z3-4.8.9-x64-win\bin;C:/Program Files/Alire\bin;C:\Users\laarpjljvd\.cache\alire\msys64\mingw64\bin"
$env:REJUVENATION_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada"
$env:TESTS_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests"
$env:XMLADA_ALIRE_PREFIX = "C:\path\to\Rejuvenation-Ada\tests\alire\cache\dependencies\xmlada_22.0.0_b322ae27"

The PATH value worries me, due to the different slash before the bin directory of the first two entries

C:\Users\laarpjljvd\.config\alire\cache\dependencies\gprbuild_22.0.1_c842bbc5/bin;
C:\Users\laarpjljvd\.config\alire\cache\dependencies\gnat_native_11.2.4_2f9c5d6d/bin;
C:\Users\laarpjljvd\.cache\alire\msys64\usr\bin;
C:\Users\laarpjljvd\.cache\alire\msys64\usr\local\bin;

Can it be made consistent, like the other two alire bin directories that are included?

Also one of the last entries, worry me

C:/Program Files/Alire\bin;
C:\Users\laarpjljvd\.cache\alire\msys64\mingw64\bin

Can it be made consistent, i.e. "C:\Program Files\Alire\bin"?

Thanks in advance, Pierre

Fabien-Chouteau commented 2 years ago

@pjljvandelaar indeed it should be all / here. What differentiate gprbuild and gnat_native crates is that they are binary crates and the PATH is set with this in the manifest:

[environment]
PATH.prepend = "${CRATE_ROOT}/bin"

@mosteo We could have a different one for Windows in the manifest using a case, or we could "fix" the path in Alire.

mosteo commented 2 years ago

We have the Portable/Native path types to try and avoid these things, so If we can be consistent that would be the better fix. I guess there's some missing conversion from portable to native.

mosteo commented 11 months ago

The PATH value worries me, due to the different slash before the bin directory of the first two entries

I've located the source of this problem in our sources and I have a fix incoming.

Also one of the last entries, worry me

This seems to happen only for the Alire Power Shell, and also with echo $env:PATH, so probably coming from our installer, will check.

mosteo commented 7 months ago

W.r.t. the C:/Program Files/Alire\bin; PATH part, I'm puzzled because the installer is clearly using backslashes here. As long as it doesn't cause any trouble, I wouldn't lose more sleep over it.

Fabien-Chouteau commented 7 months ago

@mosteo Looking at the issue with:

[environment]
PATH.prepend = "${CRATE_ROOT}/bin"

I don't see a clear solution for this. If we add a process to change / into \ on Windows, when do we apply it? We can make a special case for environment.PATH.

Another solution would be to always have the directory separator at the end of the ${CRATE_ROOT} value so the manifest would look like this.

[environment]
PATH.prepend = "${CRATE_ROOT}bin"
mosteo commented 7 months ago

I don't see a clear solution for this. If we add a process to change / into \ on Windows, when do we apply it?

That's what #1483 addresses. Basically, these are always interpreted as paths, internally stored as portable ('/') and converted accordingly when emitted. My idea was to add the possibility of escaping them somehow down the line, in case a literal '\' is ever wanted on non-Windows (can't think of a use case but...).

I don't think making PATH a particular case works, as this can be WHATEVER_PREFIX, etc, and having a few trigger words seems a recipe for confusion. Another idea was to have VARIABLE.literal.prepend (does not reformat) and VARIABLE.prepend (reformats to native paths), but I thought that was too much at this time.

Finally, after seeing that Git for windows simply uses '/' (it stores paths like C:/foo/bar) and there seems to be no ill effect, maybe we could just do the same. I should first research that Windows is indeed ok with '\' and '/' interchangeably.

For now, I think #1483 is the minimal intervention to keep things under general expectations.