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

[openimageio] Separate feature flags for tools and viewer #34556

Closed
jreichel-nvidia opened this issue Oct 18, 2023 · 5 comments · Fixed by #34699
Closed

[openimageio] Separate feature flags for tools and viewer #34556

jreichel-nvidia opened this issue Oct 18, 2023 · 5 comments · Fixed by #34699
Assignees
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist

Comments

@jreichel-nvidia
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The viewer "iv" is the only tool that depends on Qt. This pulls in a large number of dependencies, which is completely unnecessary for users that are only interested in the other tools but don't need the viewer.

Proposed solution

Add another feature flag, e.g. "viewer", that controls building of the viewer.

Since the remaining tools do not require any additonal dependencies, one could also remove the "tools" feature flag and build/install them unconditionally. (This would also solve the problem when calling vcpkg_check_features() where the "tools" OR the "viewer" feature flag should enable OIIO_BUILD_TOOLS.)

Describe alternatives you've considered

No response

Additional context

No response

@jreichel-nvidia jreichel-nvidia added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 18, 2023
jreichel-nvidia added a commit to jreichel-nvidia/vcpkg that referenced this issue Oct 24, 2023
…34556)

Add feature flag "viewer" which controls building the viewer "iv".
Drop feature flag "tools", build tools (excluding the viewer) unconditionally.
@dg0yt
Copy link
Contributor

dg0yt commented Oct 24, 2023

This would also solve the problem when calling vcpkg_check_features() where the "tools" OR the "viewer" feature flag should enable OIIO_BUILD_TOOLS.

There is no such problem.
What I see is an indication that the new feature "viewer" should depend on the feature "tools".

@jreichel-nvidia
Copy link
Contributor Author

Ok, fine for me. Is there some documentation how to express "feature A depends on feature B" in vcpkg, or an example?

@JonLiu1993
Copy link
Member

Ok, fine for me. Is there some documentation how to express "feature A depends on feature B" in vcpkg, or an example?

You can refer to this: https://github.com/microsoft/vcpkg-docs/blob/fb2987c5399286524aa9afd2f4819353cee7f3eb/vcpkg/contributing/maintainer-guide.md#do-not-use-features-to-implement-alternatives

@jreichel-nvidia
Copy link
Contributor Author

My previous question might have been a bit unspecific since it is about "feature A depends on feature B of the same port", and I can't find that in the provided documentation.

But I wonder whether this is just a special case and the usual mechanism in ports/openimageio/vcpkg.json is the right way for depencies between features of the same port, i.e., would referencing openimage[featureB] in dependencies of featureA in ports/openimageio/vcpkg.json be ok? Or is there some other mechanism for inter-port dependencies of features?

@dg0yt
Copy link
Contributor

dg0yt commented Oct 25, 2023

Your question was clear. In-port dependencies are expressed just like inter-port dependencies: You name the port and the feature(s), i.e. you get

    "viewer": {
      "description": "Build openimageio viewer",
      "dependencies": [
        "opengl",
        {
          "name": "openimageio",
          "features": [
            "tools"
          ]
        },
        {
          "name": "qtbase",
          "default-features": false
        }
      ]
    },

data-queue pushed a commit that referenced this issue Oct 28, 2023
…34699)

* [openimageio] Separate feature flags for tools and viewer (#34556)

Add feature flag "viewer" which controls building the viewer "iv".
Drop feature flag "tools", build tools (excluding the viewer) unconditionally.

* Re-add "tools" flag.
Make "viewer" flag depend on "tools" flag.

* Ensure that Qt does not get picked up by accident if the "viewer" flag is not
selected.
Make hidden dependency of theia on OpenImageIO::iv explicit.

* Fix syntax error in USE_QT option.

* Use ENABLE_IV instead of USE_QT.
Remove explicit setting of -DUSE_QT=OFF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants