Skip to content

Add MSVC compatibility adjustments - #379

Open
eunos-1128 wants to merge 31 commits into
conda-forge:mainfrom
eunos-1128:main
Open

Add MSVC compatibility adjustments#379
eunos-1128 wants to merge 31 commits into
conda-forge:mainfrom
eunos-1128:main

Conversation

@eunos-1128

@eunos-1128 eunos-1128 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This pull request addresses compatibility issues for downstream MSVC (cl.exe) consumers when building the ImageMagick package on Windows. The main focus is to ensure that the generated magick-baseconfig.h header uses MSVC-compatible definitions for the restrict keyword and ssize_t type, preventing common build errors in dependent projects. Additionally, the build number is incremented.

Windows MSVC compatibility improvements:

  • Set autoconf cache variables (ac_cv_c_restrict and ac_cv_type_ssize_t) before configuration to ensure magick-baseconfig.h defines _magickcore_restrict as __restrict (MSVC-compatible) and provides a typedef for ssize_t, avoiding build errors in downstream MSVC projects.
  • Add fallback sed commands after build to patch magick-baseconfig.h if the autoconf cache overrides did not take effect: replace __restrict__ with __restrict and define ssize_t as ptrdiff_t if it remains undefined.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@eunos-1128

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-admin

conda-forge-admin commented Jun 16, 2026

Copy link
Copy Markdown
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 (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

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

@conda-forge-admin

Copy link
Copy Markdown
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

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

@eunos-1128

eunos-1128 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@ocefpaf @halldc
Ready to be merged.

Comment thread recipe/build.sh
# semantics of ssize_t; using 'int' (32-bit) would risk overflow for
# images larger than 2GB.
sed -i 's|/\* #undef ssize_t \*/|#define ssize_t ptrdiff_t|' "${BASECONFIG}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, can you add a test in tests that does a compilation with msvc?

@eunos-1128 eunos-1128 Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor build script to handle Windows parallel installation and check.
This file tests compatibility of MagickCore headers with MSVC, checking for issues like ssize_t declaration and restrict qualifier usage.
Modify header files to change warning directives to pragma messages.
Removed obsolete compatibility warning block related to MAGICKCORE_QUANTUM_DEPTH.
@conda-forge-admin

Copy link
Copy Markdown
Contributor

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

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-admin

conda-forge-admin commented Jun 22, 2026

Copy link
Copy Markdown
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 (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

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

Modify magick-baseconfig.h handling for MSVC compatibility by adjusting attribute definitions and ensuring ssize_t typedef exists.
Add grep command to check for ssize_t in magick-baseconfig.h
@eunos-1128

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-admin

Copy link
Copy Markdown
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

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

Remove grep command that checks for ssize_t in magick-baseconfig.h during installation.
Comment thread recipe/build.sh Outdated
#
# Also, clang on Windows provides ssize_t via sys/types.h, so AC_TYPE_SSIZE_T
# finds it and emits no typedef in magick-baseconfig.h. MSVC's SDK does not
# provide ssize_t, so we inject a typedef ptrdiff_t ssize_t guarded by _MSC_VER.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems wrong. _MSC_VER is also set with clang on Windows right?

@eunos-1128 eunos-1128 Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_MSC_VER seems defined not only by cl.exe and clang-cl, but also by clang when targeting the MSVC ABI. Since the conda-forge Windows build targets the MSVC ABI, _MSC_VER seems defined during the build with clang/autotools as well.

The #ifdef _MSC_VER guard in the injected header is therefore intentional — it is meant to cover all compilers targeting the MSVC ABI (cl.exe, clang-cl, and clang with MSVC target), all of which lack ssize_t in their SDK.

The comment was misleading by implying only cl.exe. I have updated it:

https://github.com/eunos-1128/imagemagick-feedstock/blob/main/recipe/build.sh#L128-L141

Reference: https://clang.llvm.org/docs/CommandGuide/clang.html (-fmsc-version= option: "When on Windows, this defaults to either the same value as the currently installed version of cl.exe, or 1933.")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isuruf

How about this...?

@eunos-1128
eunos-1128 requested a review from isuruf June 25, 2026 06:34
Updated comments to clarify the role of expat and freetype in the build process.
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