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

Rebase on top of cf #57

Closed
wants to merge 3 commits into from

Conversation

mingwandroid
Copy link
Contributor

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 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:

  • Using toolchain directly in this manner is deprecated. Consider using the compilers outlined here.

@mingwandroid
Copy link
Contributor Author

cc @bgruening

@mingwandroid
Copy link
Contributor Author

This should be interesting, I expect failures a few times round since I've been using our compilers since 3.4.0!

@bgruening
Copy link
Contributor

@mingwandroid; With @ocefpaf and others we are considering to use R 3.5.1 as an POC for rebuilding with the new compilers. So if this fails maybe we can include a local conda_build_config.yaml here indicating the new compilers already?

@ocefpaf @isuruf what do you think?

@mingwandroid
Copy link
Contributor Author

You might need to build out the transitive c and c++ deps with them first.. shouldn't be too bad though?

- {{native}}bzip2 1.0.* # [win]
- {{native}}libjpeg-turbo # [win]
- {{native}}libiconv # [win]
- {{native}}libuuid # [linux]
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove {{native}} here? conda-build doesn't recognize that libuuid is a used variable with {{native}} in front.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then that is what needs to be fixed. Please consider a PR to conda-build to fix the real problem.

Why does it matter here though here since libuuid is not part of a build matrix?

- {{native}}gmp # [win]
- {{native}}fftw # [win]
- {{native}}xz # [win]
- {{native}}mpfr # [win]
Copy link
Member

Choose a reason for hiding this comment

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

This should be pinned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No they should not, we use run_exports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not going to infect my recipes with pinning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are wrong here anyway, msys2 packages never need pinning, they use an epoch system instead.

Copy link
Member

Choose a reason for hiding this comment

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

You are wrong here anyway, msys2 packages never need pinning, they use an epoch system instead.

Can you explain this please?

Copy link
Contributor Author

@mingwandroid mingwandroid Aug 6, 2018

Choose a reason for hiding this comment

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

Every msys2 package depends on an epoch package and that is versioned at the date when the upstream rolling distribution packages were converted to conda packages. That is the only version constraint here because that's how rolling distributions work.

Copy link
Member

Choose a reason for hiding this comment

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

Every msys2 package depends on an epoch package and that is versioned at the date when the upstream rolling distribution packages were converted to conda packages. That is the only version constraint here because that's how rolling distributions work.

Sure. How does the r-base package pin to that epoch package? Is it done by run_exports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Transitively each msys2 dependency depends on the same epoch version.

Copy link
Contributor Author

@mingwandroid mingwandroid Aug 6, 2018

Choose a reason for hiding this comment

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

Future-wise, R upstream are now based on MSYS2 (and are thus more up to date than us) so we may want to consider just bundling all their static libs into a single package and using that. We'd also need to update our mingw-w64 toolchain (or provide the 'upstream' one, just for R).

@jdblischak jdblischak mentioned this pull request Aug 7, 2018
6 tasks
@bgruening
Copy link
Contributor

New error: configure: error: C compiler cannot create executables

@bgruening
Copy link
Contributor

I guess this one is blocked by the epic rebuild of conda-forge.

@ocefpaf
Copy link
Member

ocefpaf commented Aug 14, 2018

I guess this one is blocked by the epic rebuild of conda-forge.

Sadly yes :-(

@jakirkham
Copy link
Member

Looks like the epic rebuild caught up to r-base. ( #60 ) Maybe time to revisit this one. 😉

@mingwandroid
Copy link
Contributor Author

Caught up? Yay! conflicts, bah!

Try to get this rebased tomorrow; we need to figure something out regarding CF X11 vs AD CDT/X11 for recipes.

@bgruening
Copy link
Contributor

Hi @mingwandroid, are you planning on working on this soon?

@mingwandroid
Copy link
Contributor Author

Sorry didn't get a chance. Maybe this weekend? Do you need it earlier? I'll try to find time soon if so.

@bgruening
Copy link
Contributor

@mingwandroid Its blocking currently the conda-forge rebuild. I can also give it a try and rebase it at least, if you like.

@bgruening
Copy link
Contributor

@mingwandroid I tried to rebase. Feel free to revert this commit. Lets see if this gets green :)

@bgruening
Copy link
Contributor

@conda-forge-admin, please rerender

number: 1
merge_build_host: True # [win]
merge_build_host: True # [win]
number: 0

Choose a reason for hiding this comment

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

Suggested change
number: 0
number: 2

@jakirkham
Copy link
Member

...we need to figure something out regarding CF X11 vs AD CDT/X11 for recipes.

cc @pkgw

@mingwandroid
Copy link
Contributor Author

mingwandroid commented Oct 21, 2018

@pkgw, @jakirkham, what do we want to do with things like Qt? Is it being rebuilt now on cf CI? Is it using cf X11?

r-gl from conda-forge currently links to these system (non-glibc/kernel) libraries:

ldd /opt/conda/envs/r351cf/lib/R/library/rgl/libs/rgl.so
	libglapi.so.0 => /usr/lib64/libglapi.so.0 (0x00007fe30e5dd000)
	libselinux.so.1 => /lib64/libselinux.so.1 (0x00007fe30e3be000)
	libXdamage.so.1 => /usr/lib64/libXdamage.so.1 (0x00007fe30dfa7000)
	libXfixes.so.3 => /usr/lib64/libXfixes.so.3 (0x00007fe30dda2000)
	libXxf86vm.so.1 => /usr/lib64/libXxf86vm.so.1 (0x00007fe30d348000)
	libdrm.so.2 => /usr/lib64/libdrm.so.2 (0x00007fe30d13b000)
	libexpat.so.1 => /lib64/libexpat.so.1 (0x00007fe30caf2000)

Is this more widespread? What's the official policy as regards OpenGL/mesa? End users need to install stuff here with their system package manger. Is conda-forge using the --error-overlinking flag so that these dependencies can be explicitly identified in the recipes at least?

@isuruf isuruf closed this Nov 12, 2018
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.

8 participants