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

ImageMagick: Add 7.1.0-29 and build it with cmake on all platforms. #10003

Closed
wants to merge 7 commits into from

Conversation

Cyriuz
Copy link
Contributor

@Cyriuz Cyriuz commented Mar 29, 2022

This fixes the previously broken Windows build and adds a bunch of new possible dependencies.

Specify library name and version: imagemagick/7.1.0-29

I was a bit sad when I realized that the current ImageMagick package in conan-center did not support Windows and after looking around a bit for solutions I found some previous discussions about its build scripts being very complicated for Windows and some attempts at contributing a CMake build script instead (here and here). So I continued the work on the CMake scripts to make it work with the latest release and on windows/linux/mac. The patch provided in this PR is based on my fork here.

The maintainers of ImageMagick have stated an interest in a CMake build script but would like some more people to test and use it before adapting it as stated here. So I thought this would be a good way to get it used even if it is not merged to ImageMagick itself yet, as the CMake build script simplifies the build by miles.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

This fixes the previously broken Windows build and adds a bunch of new possible dependencies.
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

CI didn't detect changes here, we need to investigate it.

cc/ @uilianries @danimtb

@conan-center-bot conan-center-bot requested a review from SSE4 April 12, 2022 12:01
@jgsogo jgsogo added the infrastructure Waiting on tools or services belonging to the infra label Apr 19, 2022
@conan-center-bot conan-center-bot requested a review from jgsogo April 20, 2022 12:01
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@danimtb danimtb removed the infrastructure Waiting on tools or services belonging to the infra label Jul 12, 2022
@stale
Copy link

stale bot commented Aug 11, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 11, 2022
@Cyriuz
Copy link
Contributor Author

Cyriuz commented Aug 31, 2022

Sorry for being slow on this, how do you usually handle missing prebuilts?

@uilianries
Copy link
Member

@Cyriuz I gonna check will cairo is missing and try to regenerate it. THanks for reporting.

@stale stale bot removed the stale label Aug 31, 2022
@conan-center-bot

This comment has been minimized.

@Cyriuz
Copy link
Contributor Author

Cyriuz commented Sep 12, 2022

@uilianries Did you get a chance to look into this? I could disable these deps by default I guess but maybe that is the wrong way to handle it :)

@uilianries
Copy link
Member

@Cyriuz yes, I tried to regenerate both cairo and pango multiple times, but that same package id is always missing. I'll investigate further tomorrow, thank you for pinging!

@jgsogo
Copy link
Contributor

jgsogo commented Sep 15, 2022

Hi! Here the issue (at least one of them) with cairo is related to glib:

  • cairo requires glib/2.73.3

  • ...but this requirement is overridden by pango:

    WARN: cairo/1.17.4: requirement glib/2.73.3 overridden by pango/1.50.7 to glib/2.73.0
    
  • and cairo recipe, on it side, has decided to depend on every single bit from glib:

    self.info.requires["glib"].full_package_mode()
    

    As a consequence, the different patch version, results in different package-id for cairo.

We just need to upgrade glib in pango to glib/2.73.3 so the version is not overridden

@uilianries
Copy link
Member

@jgsogo thank you very much!!

@ghost
Copy link

ghost commented Oct 13, 2022

I detected other pull requests that are modifying imagemagick/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot
Copy link
Collaborator

Failure in build 15 (4db101702f4c386dc6c7505d64683f9973d04292):

  • imagemagick/7.0.11-14@:
    Error running command conan info imagemagick/7.0.11-14@#57faf72cd979f014154ffc7b8dd1c857 --json {jsonName} -pr {profileName}:

    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=5
    os=Linux
    [options]
    imagemagick:shared=False
    
    ...
    WARN: openexr/2.5.7: requirement zlib/1.2.12 overridden by imagemagick/7.0.11-14 to zlib/1.2.11 
    WARN: pango/1.50.10: requirement freetype/2.12.1 overridden by imagemagick/7.0.11-14 to freetype/2.10.4 
    WARN: freetype/2.10.4: requirement libpng/1.6.38 overridden by pango/1.50.10 to libpng/1.6.37 
    WARN: freetype/2.10.4: requirement zlib/1.2.12 overridden by pango/1.50.10 to zlib/1.2.11 
    WARN: libpng/1.6.37: requirement zlib/1.2.12 overridden by freetype/2.10.4 to zlib/1.2.11 
    WARN: fontconfig/2.13.93: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    WARN: libxft/2.3.4: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    WARN: cairo/1.17.4: requirement zlib/1.2.12 overridden by pango/1.50.10 to zlib/1.2.11 
    WARN: cairo/1.17.4: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    ERROR: Conflict in cairo/1.17.4:
        'cairo/1.17.4' requires 'expat/2.4.8' while 'fontconfig/2.13.93' requires 'expat/2.4.9'.
        To fix this conflict you need to override the package 'expat' in your root package.
    
  • imagemagick/7.1.0-29@:
    Error running command conan info imagemagick/7.1.0-29@#c1c93aca3568be2432561e3c44e7fd48 --json {jsonName} -pr {profileName}:

    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=5
    os=Linux
    [options]
    imagemagick:shared=False
    
    ...
    imagemagick/7.1.0-29: WARN: Conan package for OpenMP is not available, this package will be used from system.
    WARN: pango/1.50.10: requirement freetype/2.12.1 overridden by imagemagick/7.1.0-29 to freetype/2.10.4 
    WARN: freetype/2.10.4: requirement libpng/1.6.38 overridden by pango/1.50.10 to libpng/1.6.37 
    WARN: fontconfig/2.13.93: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    WARN: libxft/2.3.4: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    WARN: cairo/1.17.4: requirement freetype/2.12.1 overridden by pango/1.50.10 to freetype/2.10.4 
    ERROR: Conflict in cairo/1.17.4:
        'cairo/1.17.4' requires 'expat/2.4.8' while 'fontconfig/2.13.93' requires 'expat/2.4.9'.
        To fix this conflict you need to override the package 'expat' in your root package.
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@Cyriuz
Copy link
Contributor Author

Cyriuz commented Oct 13, 2022

Sorry for being so slow with this and thanks for pointing out the issue. I wanted to try to bump pango but got hit by the linter now, I had hoped I wouldn't need to change the 7.0 recipe since it doesn't build on my Windows dev env but maybe its unavoidable? Not sure if it is worth fixing this PR since we now have #12371? This one makes the recipe super easy but it depends on my cmake patch which I had hoped would be adopted by imagemagick, but that might be wishful thinking since I have had so little time to spend on this lately.

@ghost ghost mentioned this pull request Oct 29, 2022
4 tasks
@stale
Copy link

stale bot commented Nov 13, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Dec 16, 2022

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

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

Successfully merging this pull request may close these issues.

5 participants