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

OSLCompiler : Check for write errors #1360

Merged

Conversation

johnhaddon
Copy link
Contributor

Description

This fixes #1352 by checking for write errors after writing the .oso file. Without it, empty files could be written without error after running out of disk space.

Tests

I have not added any new tests, although in #1352 I did provide an example script to demonstrate the problem. I've been unable to find a way of simulating an empty disk without requiring root privilege, which I presume makes it unsuitable as a test.

Checklist:

  • I have read the contribution guidelines.
  • I have previously submitted a Contributor License Agreement.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 19, 2021

CLA Signed

The committers are authorized under a signed CLA.

@johnhaddon johnhaddon closed this Apr 19, 2021
@johnhaddon johnhaddon reopened this Apr 19, 2021
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM.

I'll merge as soon as we work out the CLA.

@johnhaddon johnhaddon closed this Apr 20, 2021
@johnhaddon johnhaddon reopened this Apr 20, 2021
@johnhaddon johnhaddon closed this Apr 20, 2021
@johnhaddon johnhaddon reopened this Apr 20, 2021
@johnhaddon
Copy link
Contributor Author

Not sure what went wrong with the CLA the first time - it had been signed and I'd even received confirmation emails that I'd been added to the approved list. We started again from scratch with a new organisation and it appears to have worked this time.

@lgritz lgritz merged commit 8dafa41 into AcademySoftwareFoundation:master Apr 20, 2021
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Apr 22, 2021
johnhaddon added a commit to johnhaddon/gaffer that referenced this pull request Nov 22, 2021
Despite AcademySoftwareFoundation/OpenShadingLanguage#1360, we have still seen one example of an empty `.oso` file being produced in production. Adding further checks will hopefully help us get to the bottom of it if it happens again.
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.

OSLCompiler not checking for write failures
2 participants