conda-forge / omniorb-feedstock

A conda-smithy repository for omniorb.
BSD 3-Clause "New" or "Revised" License
0 stars 9 forks source link

Refactor multi output and add omniorbpy #53

Closed beenje closed 1 month ago

beenje commented 3 months ago

Checklist

Refactored the multi-output as current solution was quite fragile as pointed out in https://github.com/conda-forge/omniorb-feedstock/pull/46

Add omniorbpy as new output as discussed in https://github.com/conda-forge/gepetto-viewer-corba-feedstock/pull/22

This allows to:

When merged, the https://github.com/conda-forge/omniorbpy-feedstock needs to be archived.

conda-forge-webservices[bot] commented 3 months ago

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:

beenje commented 3 months ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 3 months ago

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.

beenje commented 3 months ago

@conda-forge-admin, please rerender

beenje commented 3 months ago

@lockhart I refactored the recipe multi output and included omniORBpy.

I had issues compiling the ssl transport with the latest openssl. configure reported it found openssl but compilation was skipped. Issue was that pkg-config --variable=prefix openssl reports an empty string and that was used for OPEN_SSL_ROOT. I applied a small patch to use the prefix instead (in both omniorb and omniorbpy) and opened an issue in https://github.com/conda-forge/openssl-feedstock/issues/155

omniorbpy build still raises some warning about omniorb-libs not being in reqs/run. It is in host. In run, we have {{ pin_subpackage('omniorb', exact=True) }}, which itself also as in run {{ pin_subpackage('omniorb-libs', exact=True) }}. I guess I could add it as well?

@h-vetinari , as you contributed to this recipe before, I'd appreciate some review. If you have time of course.

lockhart commented 3 months ago

Oh great! I’ve started working on this but had a dearth of examples in the conda docs and conda feedstocks to see how to split this up.

Happy to drop my work and help with testing of this new version.

On May 9, 2024, at 8:45 AM, Benjamin Bertrand @.***> wrote:

Checklist

Used a personal fork of the feedstock to propose changes https://conda-forge.org/docs/maintainer/updating_pkgs.html#forking-and-pull-requests Bumped the build number (if the version is unchanged) Re-rendered https://conda-forge.org/docs/maintainer/updating_pkgs.html#rerendering-feedstocks 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. Refactored the multi-output as current solution was quite fragile as pointed out in #46 https://github.com/conda-forge/omniorb-feedstock/pull/46 Add omniorbpy as new output as discussed in conda-forge/gepetto-viewer-corba-feedstock#22 https://github.com/conda-forge/gepetto-viewer-corba-feedstock/pull/22 This allows to:

release both omniorb and omniorbpy at the same time make omniorbpy depends on the exact version of omniorb build omniorbpy on windows (it needs to be built from under omniorb/src/lib) When merged, the https://github.com/conda-forge/omniorbpy-feedstock needs to be archived.

You can view, comment on, or merge this pull request online at:

https://github.com/conda-forge/omniorb-feedstock/pull/53

Commit Summary

aac5c9b https://github.com/conda-forge/omniorb-feedstock/pull/53/commits/aac5c9b8969f9b0e3d67be63acdfa3cb469acba1 Refactor multi output and add omniorbpy File Changes (8 files https://github.com/conda-forge/omniorb-feedstock/pull/53/files) M recipe/bld.bat https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-54cf74e113dd3f6d11e092fdb1d888ec82c69bdafbb15cfb6570c83ecad28f33 (19) M recipe/build.sh https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-d7075654874cb08007a21aaab3ecd4b3453a9087e7505d034d548b8938b599bc (9) A recipe/install_omniorb.bat https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-ac55cd251cb9aeb3cafe74bd30e446cd588e76d31585cf7f94cda366145ae249 (23) A recipe/install_omniorb.sh https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-d9aa0274ea72d8a446c256c78e1cd917ca1876d011a831526a23c320cf6d6b74 (25) A recipe/install_omniorbpy.bat https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-65ca5fccf4ecacf1f4fd6346c55590e22e1df091ed969f161c1cf7872aced47d (20) A recipe/install_omniorbpy.sh https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-853e17345f0de2159809efb7bee89f791a9bc9b425a88630a4e9598a596b5b7d (12) M recipe/meta.yaml https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-f3725a55bf339595bf865fec73bda8ac99f283b0810c205442021f29c06eea9a (159) A recipe/windows-build-omniorbpy.patch https://github.com/conda-forge/omniorb-feedstock/pull/53/files#diff-70c5699a6635e2b1a8fe2ed76150c1fe3130ea3c1d0a8c89be927bb6ad09f43b (91) Patch Links:

https://github.com/conda-forge/omniorb-feedstock/pull/53.patch https://github.com/conda-forge/omniorb-feedstock/pull/53.diff — Reply to this email directly, view it on GitHub https://github.com/conda-forge/omniorb-feedstock/pull/53, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANLDAKCOTHYR5WOVPGTOETZBOK27AVCNFSM6AAAAABHPBQCEKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4DOOJWHA2DAMA. You are receiving this because your review was requested.

github-actions[bot] commented 3 months ago

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/omniorb-feedstock/actions/runs/9035882141.

h-vetinari commented 3 months ago

omniorbpy build still raises some warning about omniorb-libs not being in reqs/run. It is in host. In run, we have {{ pin_subpackage('omniorb', exact=True) }}, which itself also as in run {{ pin_subpackage('omniorb-libs', exact=True) }}. I guess I could add it as well?

I don't know the exact reason why, but run-exports don't get applied in the recipe where the output (that should have the run-export) in question is being built. So you should add a run-dep for {{ pin_subpackage('omniorb-libs', exact=True) }}

beenje commented 3 months ago

@conda-forge-admin, please rerender

beenje commented 3 months ago

Pinging @conda-forge/omniorbpy explicitly as it impacts your feedstock.

I updated the feedstock to store artifacts on azure. To test, you can download them here

beenje commented 2 months ago

@lockhart if you are happy with this PR, please approve and I will merge

lockhart commented 2 months ago

OK done just now. Sorry for dropping the ball on this.

On Jul 5, 2024, at 2:28 AM, Benjamin Bertrand @.***> wrote:

@lockhart https://github.com/lockhart if you are happy with this PR, please approve and I will merge

— Reply to this email directly, view it on GitHub https://github.com/conda-forge/omniorb-feedstock/pull/53#issuecomment-2210532210, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANLDANLNZQ6JPBVAZYHUELZKZRM7AVCNFSM6AAAAABHPBQCEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJQGUZTEMRRGA. You are receiving this because you were mentioned.