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

Update to cmake v3.17.0 #115

Merged
merged 13 commits into from
Mar 27, 2020
Merged

Update to cmake v3.17.0 #115

merged 13 commits into from
Mar 27, 2020

Conversation

trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Mar 23, 2020

Checklist

  • Used a 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.

@conda-forge-linter
Copy link

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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@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.

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

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Paul! 😄

One comment on patches that we can follow up on after CI reports. Otherwise looks good.

recipe/meta.yaml Outdated Show resolved Hide resolved
@jakirkham jakirkham mentioned this pull request Mar 23, 2020
5 tasks
@jakirkham
Copy link
Member

Restarted CI as it appeared something funky was going on.

@jakirkham
Copy link
Member

jakirkham commented Mar 24, 2020

I wonder if we are seeing some kind of conda-build bug. It appears to be checking if macOS is used in the Linux build (also full CI text log).

WARNING :: get_rpaths_raw()=['/home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_build_env/lib:/home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib'] and patchelf=[['/home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_build_env/lib', '/home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib']] disagree for /home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/bin/cmake :: 
Warning: rpath /home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_build_env/lib is outside prefix /home/conda/feedstock_root/build_artifacts/cmake_1585077223713/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place (removing it)
Traceback (most recent call last):
  File "/opt/conda/bin/conda-build", line 11, in <module>
    sys.exit(main())
  File "/opt/conda/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 474, in main
    execute(sys.argv[1:])
  File "/opt/conda/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 465, in execute
    verify=args.verify, variants=args.variants)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/api.py", line 209, in build
    notest=notest, need_source_download=need_source_download, variants=variants)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 2868, in build_tree
    notest=notest,
  File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 2155, in build
    newly_built_packages = bundlers[pkg_type](output_d, m, env, stats)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 1509, in bundle_conda
    files = post_process_files(metadata, initial_files)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 1368, in post_process_files
    post_build(m, new_files, build_python=python)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/post.py", line 1174, in post_build
    post_process_shared_lib(m, f, prefix_files, host_prefix)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/post.py", line 1122, in post_process_shared_lib
    mk_relative_osx(path, host_prefix, m.config.build_prefix, files=files, rpaths=rpaths)
  File "/opt/conda/lib/python3.7/site-packages/conda_build/post.py", line 399, in mk_relative_osx
    assert sys.platform == 'darwin'
AssertionError

Edit: Asking about this on Gitter.

@jakirkham
Copy link
Member

FWIW that discussion has led to PR ( conda/conda-build#3912 ).

@mingwandroid
Copy link
Contributor

Do we know which file this is or how Linux may be creating a macOS binary?

@jakirkham
Copy link
Member

It's share/cmake-3.17/Modules/Internal/CPack/CPack.OSXScriptLauncher.in. I think it is already in the source distribution.

cc @chrisburr (in case I'm missing anything 🙂)

@mingwandroid
Copy link
Contributor

Thanks, we should implement a way to explicitly disable RPATH fixup for specific files I think.

@jakirkham
Copy link
Member

That would be helpful 🙂

What would you recommend we do in the interim?

Also do you have any thoughts on the patches above? We dropped them as we figured they got upstreamed, but please let us know if that is not the case and we should do something else.

@mingwandroid
Copy link
Contributor

Well we need to build cmake so lgtm.

@chrisburr
Copy link
Member

Well we need to downgrade conda-build for this to work. I suggested editing .scripts/build_steps.sh to replace conda-build with 'conda-build<3.18'.

.scripts/build_steps.sh Outdated Show resolved Hide resolved
@jakirkham
Copy link
Member

jakirkham commented Mar 26, 2020

Seeing a weird error on Windows. Maybe the package for pkginfo is broken?

ERROR conda.core.link:_execute(568): An error occurred while installing package 'conda-forge::pkginfo-1.5.0.1-py_0'.
CondaError: Cannot link a source that does not exist. C:\Miniconda\Scripts\conda.exe
Running `conda clean --packages` may resolve your problem.
Attempting to roll back.

Also here's the log on CI and raw text log for download.

Edit: Raised on Gitter.

@jakirkham
Copy link
Member

To fix the AppVeyor issue, we need to re-render it seems. However that would wipe out the changes here. So I requested the bot create PR ( #117 ) with the re-render. Though of course that needed the fixes from here. 😂 So I wound up cherry-picking the fixes into that PR. Once that PR passes will merge and then merge the other update PR ( #114 ) and this PR (finally shaving the yak 😉).

@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.

conda-forge-linter and others added 2 commits March 27, 2020 00:20
@jakirkham jakirkham merged commit fc0e2d6 into conda-forge:master Mar 27, 2020
@jakirkham
Copy link
Member

Thanks @trxcllnt for the PR and everyone for the reviews! 😄

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.

5 participants