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 : Race condition when building Cortex (with many threads or in debug mode) #1309

Closed

Conversation

boberfly
Copy link
Contributor

Generally describe what this PR will do, and why it is needed.

Related Issues

Dependencies

  • NA

Breaking Changes

  • NA

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Cortex project's prevailing coding style and conventions.

…cies/rl9/lib/libIECorePython.so: file too short
@johnhaddon
Copy link
Member

As I described in the previous PR, I don't think this is the right approach - we don't want to make the build depend on the install. We should be able to build, test and only then install. Does my description of the problem in #1185 (comment) apply in your case Alex? I think the solution is to ensure that the -L./lib for the build dir comes before the -L for the dependencies dir...

@boberfly
Copy link
Contributor Author

I'm not too familiar with SConstruct so I don't really know here how to solve that one. I tried putting in any prepends I found with an additional "./lib" to ensure it is first but it didn't seem to work for me...

@johnhaddon
Copy link
Member

When it didn't work for you, was that because ./lib wasn't coming first on the command line (i.e. the SConstruct still had problems) or because ./lib was coming first but it didn't stop the linker erroring?

@boberfly
Copy link
Contributor Author

By the looks of it the BUILD_DIR is coming in first before the ./lib for those ones having issues.

@johnhaddon
Copy link
Member

OK, then I think that's the issue we need to fix.

johnhaddon added a commit to johnhaddon/dependencies that referenced this pull request Feb 7, 2023
johnhaddon added a commit to johnhaddon/dependencies that referenced this pull request Feb 9, 2023
@johnhaddon
Copy link
Member

I'm going to close this. As discussed in the comments, I don't think this is the right approach, and the urgency has also now been reduced since we added a workaround in GafferHQ/dependencies, where the problem crops up most.

@johnhaddon johnhaddon closed this Mar 2, 2023
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