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

Attempt to fix the Win32 build. #18

Merged
merged 3 commits into from
Jan 12, 2017
Merged

Conversation

pkgw
Copy link
Contributor

@pkgw pkgw commented Jan 10, 2017

Commentary to follow, if this actually works.

@pkgw
Copy link
Contributor Author

pkgw commented Jan 10, 2017

OK, well, I'll write up some notes on what seems to be going on here.

First, there seems to be a disagreement between the Cygwin settings in the Miniconda distribution, and in the Cygwin tools that are installed during the conda-build process. The Miniconda version of cygpath maps Windows drives to Unix ones with a special fake prefix of /cygdrive:

> c:\Miniconda3\Library\usr\bin\cygpath.exe c:\Miniconda3
/cygdrive/c/Miniconda3

The cygpath inside the build process doesn't have the fake prefix:

> c:\Miniconda3\conda-bld\recipe_NNNNN\_b_env\Library\usr\bin\cygpath.exe c:\Miniconda3
/c/Miniconda3

This becomes relevant because the relevant programs later map these Unix paths back into Windows paths. Some bld.bat scripts use the cygpath program to make Unix paths out of Windows paths; if those are run using the Miniconda cygpath (which I believe happens unless %PATH% gets futzed with), Cygwin tools inside the build environment will not un-map them correctly.

Second, the particular build process used for cairo is a bit tricky. The bld.bat script launches make, which then invokes the Windows C/C++ compiler cl. MSYS2 has special logic so that when a Unix-native program like make invokes a Windows-native program like cl, any paths in the program's command line arguments are mapped back to Windows-type paths. The environment variable MSYS2_ARG_CONV_EXCL controls which arguments are affected by this substitution. For reasons I don't fully understand, in my tests, clearing this variable got the Cairo build to work again.

Finally, I'm not sure exactly when it happened, but I think that a somewhat recent version of conda-build added a command to automatically set MSYS2_ARG_CONV_EXCL in the Windows build script preamble. I haven't looked up exactly when that happened but something came up when I googled — gotta run so can't find it again right now.

Also, error out earlier while we're at it.
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@pkgw
Copy link
Contributor Author

pkgw commented Jan 11, 2017

For reference, here are relevant the commits in the Git history of conda-build that alter text relating to the MSYS2 magic variables:

7c20b85 revert MSVC disutils changes
829eb86 revert MSVC disutils changes
948d7c2 add --no-activate option to bypass activation
565e5f5 add another MSYS2 exclusion
90818eb move MSYS2 exclude stuff higher, so that it doesn't reset error level
b577fa7 dumb extra character in MSYS2 exclude var setting
2286cf7 change slash-to-dash to MSYS2_ARG_CONV_EXCL

The first commit showed up on May 5, 2016, just before the 1.20.2 release.

@pkgw
Copy link
Contributor Author

pkgw commented Jan 11, 2017

Aha! The trailing semicolon in the definition of MSYS2_ARG_CONV_EXCL seems to be the culprit here. I have an example on the VM I'm using to test where I can put it in and take it out, and the build succeeds or fails depending on whether it's present (failure) or absent (success).

I suspect that MSYS2 is parsing the variable into a string of zero length, which then matches against every command-line argument, making it decide to convert no command-line arguments to any Windows programs. I'll file this as a conda-build bug since that's not the intended behavior; the trailing semicolon comes from expanding a possibly-absent previous value of the variable.

pkgw added a commit to pkgw/conda-build that referenced this pull request Jan 11, 2017
…XCL%

The previous method of setting the %MSYS2_ARG_CONV_EXCL% environment variable
tried to preserve any preexisting value of the variable. Not only does this
make the build process depend on the initial environment more, it generally
leads to a trailing semicolon in the variable value. This seems to cause MSYS2
to parse the variable as having an empty value that prevents *all* argument
conversions, as investigated in the conda-forge `cairo` package:
conda-forge/cairo-feedstock#18.

I do not know what fallout this change will have for existing build scripts,
but the current behavior cannot be what was intended.
@pkgw
Copy link
Contributor Author

pkgw commented Jan 11, 2017

There's some related discussion in this conda-build pull request.

@ccordoba12
Copy link
Contributor

Great job, this is passing now!

@ccordoba12
Copy link
Contributor

Could you also remove my suggested hack

https://github.com/conda-forge/cairo-feedstock/blob/master/appveyor.yml#L83

as part of this PR?

Revert 83dba28 and tidy up the fix now that the culprit has been identified.
@pkgw pkgw merged commit 3cf94d8 into conda-forge:master Jan 12, 2017
@pkgw
Copy link
Contributor Author

pkgw commented Jan 12, 2017

The CI failures were because I accidentally pushed my branch to the conda-forge user's GitHub account and somehow the two sets of checks get combined, and then I deleted the conda-forge branch which made the long-lag CI builds fail. So, I've gone ahead and merged.

@pkgw pkgw deleted the pr-fix-win32 branch January 12, 2017 13:20
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

Successfully merging this pull request may close these issues.

3 participants