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

Adding cf-m2-build-bat to facilitate using bash scripts in conda build on win-64 #28882

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

owinebar
Copy link

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 21, 2025

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

I wanted to let you know that I linted all conda-recipes in your PR (recipes/cf-m2-build-bat/meta.yaml) and found some lint.

Here's what I've got...

For recipes/cf-m2-build-bat/meta.yaml:

  • ❌ The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'test', 'about'].
  • ❌ The recipe could do with some maintainers listed in the extra/recipe-maintainers section.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12880208780. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Contributor

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 (recipes/cf-m2-build-bat/meta.yaml) and found it was in an excellent condition.

@owinebar
Copy link
Author

With scripts like these I was able to build emacs-feedstock on win-64 working with the build.sh.
This cleans those up and packages them for general use, i.e. anyone wanting to use shell scripts running make, etc, under conda build, without hacking conda-build itself.

@mfansler
Copy link
Member

Perhaps this should be proposed as a new example recipe?

CC: @isuruf

@isuruf
Copy link
Member

isuruf commented Jan 21, 2025

@owinebar
Copy link
Author

These scripts have the advantage that the user does not need to know any details of conda build. Further, any changes to conda build or the MSYS2 implementation won't require changes to every recipe that uses m2-* packages in the build environment.

With respect to packages already building with the m2 environment, have the installation conventions been discussed or decided in some forum? Using the MSYS root directory as the installation prefix has negative consequences in that the MSYS DLL masks the bin subdirectory of the root directory. Has there been any agreement to use the UCRT environment uniformly, as opposed to the MINGW64 environment (which also, in fact, uses UCRT)? I ask because the git package for win-64 is just a repackaging of fit for windows, which uses the MINGW64 environment.

@owinebar
Copy link
Author

@conda-forge/help-c-cpp Ready for review
I'm not sure this is the right team, as there is no C or C++ code, but many projects using the make build system are C/C++ projects, so it seems like the best option.

@owinebar
Copy link
Author

I added an argument to the trampoline scripts to specify MSYSTEM explicitly, record PREFIX from conda-build as HOST_PREFIX, then set PREFIX to MSYSTEM_PREFIX before invoking build.sh. This should give behavior more aligned with the MSYS2 build.

@owinebar
Copy link
Author

Here's an example recipe https://github.com/conda-forge/gmp-feedstock/blob/main/recipe/bld.bat

Hi, @isuruf,
It took me a while to review this example, and why it might be working.

  1. The author has manually recreated the relevant steps normally generated by conda build in bld.bat.
  2. The author was careful to append the contents of build sh to the win-build.sh script, rather than exec'ing build.sh or "bash -c build.sh", which likely would have suffered many mysterious errors due to the duplicated msys-2.0.dll on the path.
  3. I don't understand the significance of specifying the c compiler to be GCC in conda_build_config.yaml versus using the 'm2w64_c' compiler argument. Or why the C++ configuration option is turned off under Windows since g++ is available there as well
  4. Is the fact that the weren't any weird errors while running configure or make from win-build.sh just good luck or by design?

The last one I ask because when I was attempting to get emacs compiling by running build.sh, the GCC activation script complained about an invalid sed script, and the complaints were from some backslashes that were present in the text of the activation script not being seen by sed in the expression provided to it as a command line argument. AFAICT, that phenomenon is from the multiple instances of the library on the path, as the sed executable is linking to a different instance of the MSYS library than the shell process executing it.
The trampoline approach I've implemented is designed to safely avoid such mysterious failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants