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

SConstruct : Export node definitions using ExtensionAlgo #5701

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

johnhaddon
Copy link
Member

This allows us to implement core Gaffer nodes using a Box-authoring workflow, before exporting them as first-class (Python) node definitions using ExtensionAlgo at build time. ExtensionAlgo is still a little rough around the edges, but I think there's a lot of potential in using it for core nodes - I have a reasonably well featured ContactSheet node to follow using this mechanism.

And convert Anaglyph node to demonstrate this in action.

There is a slight annoyance here in that the extensions depend on the whole of `buildCore`,
because we need to be able to run Gaffer to export them. Which means they get re-exported
when anything changes. This is pretty fast anyway, but it can be avoided around by using
`scons buildCore` rather than `scons build` when you know extensions don't need to be
exported again.
- We can't let `NamedTemporaryFile` delete the file for us, because the file needs to be closed for us to be able to open it in `gaffer env python` on Windows, and calling `close()` also deletes. We can do better in Python 3.12 by passing `close_on_delete = False` and then letting the context manager delete the file on exit.
- We need to output file paths into the `exportScript` as raw strings, so that Windows `\` characters don't get interpreted as escape characters.
- We need to specify the full path to `gaffer.cmd` in `check_call()`.
@johnhaddon
Copy link
Member Author

Pushed a commit which hopefully fixes the Windows build errors.

Copy link
Contributor

@murraystevenson murraystevenson left a comment

Choose a reason for hiding this comment

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

LGTM. We spoke offline about the change in nodeGadget:color demonstrated in the new Anaglyph node due to it now deriving from SubGraph, but we're happy to tackle that at a later date.

@johnhaddon johnhaddon merged commit 84cdfd2 into GafferHQ:main Mar 5, 2024
4 of 5 checks passed
@johnhaddon johnhaddon deleted the extensionAlgoBuild branch March 15, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants