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

Build for ogre 1.10 and 1.12 #8

Merged
merged 6 commits into from
Mar 23, 2021
Merged

Build for ogre 1.10 and 1.12 #8

merged 6 commits into from
Mar 23, 2021

Conversation

Tobias-Fischer
Copy link
Contributor

@Tobias-Fischer Tobias-Fischer commented Mar 22, 2021

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.

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

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you but ran into some issues, please ping conda-forge/core
for further assistance. You can also try re-rendering locally.

@Tobias-Fischer
Copy link
Contributor Author

I think this is good to go as soon as quay is back up. @wolfv do you want to have a look whether the pinnings look fine?

@Tobias-Fischer
Copy link
Contributor Author

@traversaro what's the idea to deal with downstream packages (libignition-gui4 and libignition-sensors4)? Do a similar treatment and build them with two variants?

For context, this PR caused the PR here.

@traversaro
Copy link
Contributor

traversaro commented Mar 23, 2021

@traversaro what's the idea to deal with downstream packages (libignition-gui4 and libignition-sensors4)? Do a similar treatment and build them with two variants?

I need to check ignition-rendering, but if the OGRE headers are not included as part of the public headers of ignition-rendering, probably there is no reason to have two builds of downstream libraries?

@traversaro traversaro closed this Mar 23, 2021
@traversaro traversaro reopened this Mar 23, 2021
@traversaro
Copy link
Contributor

@traversaro what's the idea to deal with downstream packages (libignition-gui4 and libignition-sensors4)? Do a similar treatment and build them with two variants?

I need to check ignition-rendering, but if the OGRE headers are not included as part of the public headers of ignition-rendering, probably there is no reason to have two builds of downstream libraries?

I checked, and while ignition-rendering installs some headers with ogre neither ignition-gui nor ignition-sensors use them, so in theory the build should work fine with both ogre 1.10 and 1.12 . For downstream libraries such as ignition-sensors that seems to have runtime problems with ogre 1.12, what I was thinking to do is to have a run_constrained requirement ( https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#run-constrained ) on ogre.

@traversaro
Copy link
Contributor

I checked, and while ignition-rendering installs some headers with ogre neither ignition-gui nor ignition-sensors use them, so in theory the build should work fine with both ogre 1.10 and 1.12 .

However, this is just a theory, and we could have subtle problems related to this, so a safer alternative could be to just depend explicitly on ogre and build the two variants for all downstream libraries (this would have the advantage that we explicitly run the test suite against both ogre 1.12 and 1.10 and we easily detect problems).

@traversaro
Copy link
Contributor

In any case, I think this PR is good to go.

@Tobias-Fischer
Copy link
Contributor Author

Agreed - shall we merge @wolfv?

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.

4 participants