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

Input file with absolute path should not disable -working-directory option #115

Closed
wants to merge 1 commit into from

Conversation

Qining
Copy link
Contributor

@Qining Qining commented Feb 1, 2016

Previous glslc ignore -working-directory when the input files are passed with absolute paths, the output files are placed with the input files. But clang actually respects working-directory in the same cases.

  1. Whenever 'linking' is disabled, -working-directory should have effects.
  2. The output file location should refer to -working-directory, except when linking is enabled.


shader = FileShader('void main() {}', '.vert')
environment = Directory('.', [
Directory('subdir', []),
])
glslc_args = ['-c', '-working-directory=subdir', shader]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous test expect to find the output file in the same directory with the input shader file. But we should expect the output file in 'subdir'.

if (!needs_linking_ && !workdir_.empty()) {
if (shaderc_util::IsAbsolutePath(output_file)) {
// Get the file name from the absolute path of the input file.
output_file = GetBaseNameFromAbsolutePath(output_file);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems really confusing.

@dneto0
Copy link
Collaborator

dneto0 commented Feb 1, 2016

Seems the blurb at GetOutputFileName is very underspecified. The glslc/README.asciidoc also leaves some holes.

@dneto0
Copy link
Collaborator

dneto0 commented Feb 1, 2016

Here's my attempt at spelling out exactly how to compute the output file name.
I'd like @dekimir and @AWoloszyn to weigh in on it.

dneto0@92a7d3d

if (!needs_linking_ && !workdir_.empty() &&
!shaderc_util::IsAbsolutePath(input_filename)) {
if (!needs_linking_ && !workdir_.empty()) {
if (shaderc_util::IsAbsolutePath(output_file)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems wrong. Shouldn't it be ! ... IsAbsolutePath?

@dneto0
Copy link
Collaborator

dneto0 commented Feb 2, 2016

Please see proposed comment in #119 for how I'd like it to all work.
And yes, there are missing cases in our test suite. So those should be added too.

…vior when input file names are represented in absolute paths.
@dneto0
Copy link
Collaborator

dneto0 commented Feb 8, 2016

We've removed -working-directory, so this PR should be reworked or dropped. :-)

@Qining
Copy link
Contributor Author

Qining commented Feb 8, 2016

Working directory has been removed from glslc in commit: 7b94f85.

So this PR is closed.

@Qining Qining closed this Feb 8, 2016
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.

None yet

2 participants