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 : Ensure gaffer wrapper exists before calling usdGenSchema #5838

Merged

Conversation

murraystevenson
Copy link
Contributor

Noticed this while running a clean build where GafferArnold was being built at the same time. The Arnold schema was attempting to generate via gaffer env usdGenSchema before the gaffer wrapper was installed.

Not an issue on CI as we build Gaffer first and then the various GafferArnolds...

Copy link
Member

@johnhaddon johnhaddon 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 catching that Murray - made a suggestion inline that with a bit of luck might allow us to simplify things even further.

SConstruct Outdated
@@ -1879,6 +1880,7 @@ for libraryName, libraryDef in libraries.items() :
schemaSource,
buildSchema
)
env.Depends( generatedSchema, "additionalFiles" )
Copy link
Member

Choose a reason for hiding this comment

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

I think we also depend on the env app and the Gaffer module being installed, neither of which are captured by additionalFiles. But maybe we don't even need to use gaffer env to run usdGenSchema? It looks like commandEnv already has LD_LIBRARY_PATH etc set, so maybe we can just call usdGenSchema directly, without any extra dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! commandEnv has almost everything we need, though it's missing $BUILD_DIR/python in the PYTHONPATH and without it usdGenSchema can't find pxr.

I pushed a fixup in cfd44d8 calling usdGenSchema directly from a new env with pxr on PYTHONPATH, though maybe it's worth updating commandEnv directly for any other future use-cases?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think you're right - we might as well get commandEnv set up fully rather than make another env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I've done the commandEnv update and squashed it in.

Copy link
Member

Choose a reason for hiding this comment

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

Nice one, thanks!

This is a little simpler than using `gaffer env usdGenSchema` and solves an issue where the `gaffer` wrapper didn't exist when calling `gaffer env usdGenSchema` from a clean build.
@johnhaddon johnhaddon merged commit 8d8119c into GafferHQ:1.4_maintenance May 3, 2024
5 checks passed
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.

2 participants