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

Configure packages on Windows no longer work with autoconf >= 2.70 #7494

Closed
Mistuke opened this issue Jul 25, 2021 · 14 comments · Fixed by #7510
Closed

Configure packages on Windows no longer work with autoconf >= 2.70 #7494

Mistuke opened this issue Jul 25, 2021 · 14 comments · Fixed by #7510

Comments

@Mistuke
Copy link
Collaborator

Mistuke commented Jul 25, 2021

Describe the bug
Ever since autoconf 2.70 all configure based packages are broken on Windows with

configure: error: cannot find required auxiliary files: config.guess config.sub

(This is why I hate configure based packages). This autoconf introduced a change
in how it finds auxilary tools/scripts [4].

This breaks packages like network and ghc itself [1][2].

There's un upstream bug-report [3] but this is unlikely to be fixed.

A number of things go wrong here:

Cabal calls configure with Windows paths as arguments, e.g.:

"C:\tools\msys64\usr\bin\sh.exe" "C:\/Users/xbox/source/repos/network/configure"

But because the default PATH separator is :, so autoconf is interpeting this as:

C/
\/Users/xbox/source/repos/network/configure

Both of which are incorrect. cabal should be passing a Unix path here as configure
is a unix tool. Passing a unix path won't entirely fix the issue however
because during the call a couple of ENV variables are passed to configure which contain
Windows paths such as:

"ACLOCAL_PATH","C:\\tools\\msys64\\mingw64\\share\\aclocal;C:\\tools\\msys64\\usr\\share\\aclocal"

So the easiest fix for both issues is that cabal sets the environment variable:

env PATH_SEPARATOR=";" 

before the call to sh.

To Reproduce
Steps to reproduce the behavior:

On a checked out version of network or any other configure based package

$ cabal build --enable-tests --disable-benchmarks all

Expected behavior
For configure packages to work. We'll likely need a release of cabal-install for this.

System information

  • Windows 10
  • cabal, version 3.2.0.0

Additional context
[1] https://gitlab.haskell.org/ghc/ghc/-/issues/19189
[2] haskell/network#502
[3] https://savannah.gnu.org/support/index.php?110448
[4] http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=blob_plain;f=NEWS;hb=97fbc5c184acc6fa591ad094eae86917f03459fa

@jneira
Copy link
Member

jneira commented Jul 26, 2021

Is there any workaround to help users until we fix it? using an older msys installation? downgrade autoconf?

@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 26, 2021

Is there any workaround to help users until we fix it? using an older msys installation? downgrade autoconf?

I suppose they can downgrade to 2.69 while it's still available. They would need to manually do it as pacman doesn't support downgrades.

They can also just set PATH_SEPARATOR=";" before cabal. e.g. env PATH_SEPARATOR=";" cabal <...>

@phadej
Copy link
Collaborator

phadej commented Jul 28, 2021

What tools set ACLOCAL_PATH, that string isn't mentioned in Cabal source code.

I'd avoid using env PATH_SEPARATOR=";" it feels it could break something else

@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 28, 2021

What tools set ACLOCAL_PATH, that string isn't mentioned in Cabal source code.

ACLOCAL_PATH is set in the environment block when aclocal is installed. When cabal is called the msys2-runtime converts the environment block to contain windows paths since Cabal is a Windows program and expect a Windows style environmental block.

I'd avoid using env PATH_SEPARATOR=";" it feels it could break something else

imho : was always wrong since no Windows environment block contains : as path separator given that drive letters are separated by :. cabal just got away with it before because it wasn't causing a fatal error at startup. PATH_SEPARATOR was specifically created to support non-Posix paths [1]

So if you don't want to use it, then cabal must ensure it never passes a single non-posix path anywhere.. The problem with that is that any computed paths from the output of configure that end up in the sources of a Haskell program will be wrong.

You can also avoid this specific error by passing it the build triples explicitly --target=.. --host=... --build=.. so it no longer has to guess. But then you've also hardcoded them. (This is the solution taken for GHC as the target stays constant).

So imho, there's no solution without a singe fallout, and of everything at least PATH_SEPARATOR=";" follows a documented behavior. Which yes, will break for anyone doing path globbing incorrectly in the configure file, but those would have been wrong today already.

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.61/html_node/File-System-Conventions.html#File-System-Conventions

@pjljvandelaar
Copy link

Tried setting PATH_SEPARATOR in powershell,

PS C:\TorXakis\TorXakis.RefactorAll> $env:PATH_SEPARATOR = ";"
PS C:\TorXakis\TorXakis.RefactorAll> $env:PATH_SEPARATOR
;

but did not seem to work there...

@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 29, 2021

If you're referring to the error in commercialhaskell/stackage#6136

network             > 18796348 [main] sh 12792 fork: child -1 - forked process 17048 died unexpectedly, retry 0, exit code 0xC0000142, errno 11
network             > C:\/Users/laarpjljvd/AppData/Local/Temp/stack-9e41b18ae0723c27/network-3.1.2.2/configure: fork: Resource temporarily unavailable
network             > 18917926 [main] sh 2492 C:\Users\laarpjljvd\AppData\Local\Programs\stack\x86_64-windows\msys2-20150512\usr\bin\sh.exe: *** fatal error in forked process - fork: can't reserve memory for parent stack 0xC00000 - 0xE00000, (child has 0x440000 - 0x640000), Win32 error 487
network             > 18933826 [main] sh 2492 cygwin_exception::open_stackdumpfile: Dumping stack trace to sh.exe.stackdump
network             > 19146816 [main] sh 11648 fork: child -1 - forked process 2492 died unexpectedly, retry 0, exit code 0x100, errno 11

I think that's a different issue. In your case your sh is dying with a resource issue. It never gets to configure.

Looking at your PATH it says you're using msys2 from 2015, which if that is the case you're using a beyond ancient msys2 which has a resource bug https://sourceware.org/legacy-ml/cygwin/2016-01/msg00098.html.

Update your msys2.

@jneira jneira changed the title Configure packages on Windows no longer work. Configure packages on Windows no longer work with autoconf >= 2.70 Jul 29, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Aug 2, 2021

So the easiest fix for both issues is that cabal sets the environment variable:

env PATH_SEPARATOR=";" 

before the call to sh.

OK, let's do that. Any volunteers?

What's the best way to set the env var? Globally, the sooner the better, at cabal-install startup? In the following wrapper only?

Cabal/src/Distribution/Simple/Program/Script.hs--- | Generate a Windows batch file that invokes a program. 
Cabal/src/Distribution/Simple/Program/Script.hs-invocationAsBatchFile :: ProgramInvocation -> String

Or perhaps in any of the below as well?

Cabal/src/Distribution/Simple/Program/Script.hs--- | Generate a POSIX shell script that invokes a program.
Cabal/src/Distribution/Simple/Program/Script.hs-invocationAsShellScript :: ProgramInvocation -> String

cabal-install/src/Distribution/Client/SetupWrapper.hs-internalSetupMethod :: SetupRunner
cabal-install/src/Distribution/Client/SetupWrapper.hs-internalSetupMethod verbosity options bt args = do

@Mistuke
Copy link
Collaborator Author

Mistuke commented Aug 2, 2021

What's the best way to set the env var? Globally, the sooner the better, at cabal-install startup? In the following wrapper only?

I would say when making the call to sh only when calling configure. This will keep any other behavior as before and minimize chances of accidental breakage.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 5, 2021

Nobody volunteers, so let me take this. I assume, we are only interested in direct calls of literally a "configure" script and that we ignore the case inside https://github.com/haskell/cabal/blob/master/Cabal/src/Distribution/Make.hs (I suspect nobody uses that and certainly not on Windows), which leaves us only with https://github.com/haskell/cabal/blob/master/Cabal/src/Distribution/Simple.hs#L673

@Mikolaj Mikolaj self-assigned this Aug 5, 2021
Mikolaj added a commit to Mikolaj/cabal that referenced this issue Aug 5, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Aug 5, 2021

Could somebody test #7510 on Widows with new autoconf? I implemented it as @Mistuke recommended and haven't noticed any traps, so it should just work and [edit: not] break anything else (CI pending, sluggish today).

@jneira jneira linked a pull request Aug 5, 2021 that will close this issue
3 tasks
@Mikolaj
Copy link
Member

Mikolaj commented Aug 6, 2021

To be precise, it would be great to test the PR on Windows both with new autoconf and old autoconf (in case it breaks the old one).

Mikolaj added a commit to Mikolaj/cabal that referenced this issue Aug 6, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Aug 6, 2021

@jneira made a series of test with network and process, but setups vary, so last chance to contribute to this effort and perhaps find a breakage before PR is merged, if any of you would be so kind.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Aug 6, 2021

I'll give it some tests this weekend (sorry mostly have time for non-work things in the weekend :)) , I'll also pull the branch into ghc and build the compiler with it.

Thanks for doing this @Mikolaj!

Mikolaj added a commit to Mikolaj/cabal that referenced this issue Aug 7, 2021
Mikolaj added a commit that referenced this issue Aug 7, 2021
Set PATH_SEPARATOR=; when calling ./configure; fix #7494
@Mikolaj
Copy link
Member

Mikolaj commented Aug 7, 2021

@Mistuke: the PR is now on branch master (but not 3.6 yet). Thank you for the solution you suggested and do let me know if anything is missing.

mergify bot pushed a commit that referenced this issue Aug 23, 2021
Mikolaj added a commit that referenced this issue Aug 23, 2021
Mikolaj added a commit to Mikolaj/cabal that referenced this issue Sep 15, 2021
Mikolaj added a commit to Mikolaj/cabal that referenced this issue Sep 15, 2021
mergify bot pushed a commit that referenced this issue Sep 18, 2021
This reverts commit 70f411d.

See #7649.

(cherry picked from commit 37ccc45)
jneira pushed a commit that referenced this issue Sep 18, 2021
This reverts commit 70f411d.

See #7649.

(cherry picked from commit 37ccc45)
jneira pushed a commit that referenced this issue Sep 19, 2021
This reverts commit 70f411d.

See #7649.

(cherry picked from commit 37ccc45)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants