Need to return to working directory after checking if path exists #586
Conversation
|
Hi @RemiArnaud maybe I should have been clearer. It sounds like you think the issue is not returning to the working directory when the function ends. This is not the case. The issue is that the path is not traversed properly within the createDirectoryRecursive function. The paths being checked are relative to the original working directory, but the directory is changed by the existence check if it exists which breaks the subsequent checks. You can fix this by splitting the path into parts or (what I did here) just make sure to go back to the original working directory before checking the next path. You can see this issue if you use the latest COLLADA2GLTF on Windows targeting an output directory more than a level deep that doesn't exist yet since it uses this function. This pull request was created to resolve this regression in COLLADA2GLTF from switching to OpenCOLLADA's path implementation and will be fixed once it is merged. Let me know if you need anything else clarified here. |
| @@ -349,12 +351,14 @@ namespace COLLADABU | |||
| for (std::list<String>::const_iterator iPath = paths.begin(); iPath != paths.end(); ++iPath) | |||
| { | |||
| // if path exists | |||
| if (_chdir((*iPath).c_str()) == 0) | |||
| if (_chdir((*iPath).c_str()) == 0) { | |||
| _chdir(currentPath); | |||
There was a problem hiding this comment.
please adopt original indentation, put a few tabs to align this statement
| @@ -298,12 +298,14 @@ namespace COLLADABU | |||
| for (std::list<WideString>::const_iterator iPath = paths.begin(); iPath != paths.end(); ++iPath) | |||
| { | |||
| // if path exists | |||
| if (_wchdir((*iPath).c_str()) == 0) | |||
| if (_wchdir((*iPath).c_str()) == 0) { | |||
| _wchdir(currentPath); | |||
|
SUCCESS: exercise-opencollada-pull-request build #158
Pull Requests:
|
|
Spaces in my changes have been changed to tabs as requested. I appreciate wanting consistency, but I think it's worth mentioning that the code in |
I did not realize this. files in OpenCOLLADA\COLLADABaseUtils\src have a mixture of spaces and tabs indeed. You are right that it should all be replaced by tabs. Do you want to do this now? |
|
@RemiArnaud, I'll open up a separate pull request for normalizing formatting I think if that's something you'd like for me to do. |
|
@lasalvavida - there is also an issue with line ending apparently - #581 this PR 581 did not go through and is now with conflicts, so maybe you can address both tabs and line endings in a new RP? I should hold on merging anything in the mean time |
Currently this function does not work - it splits the path up as a list of paths for example:
/my/path/herebecomes
[/my, /my/path, /my/path/here]but it changes the working directory when it checks if the path exists. So after it checks if
/myexists, it will then check if/my/my/pathexists because the working directory has been changed to/myduring the existence check rather than checkingmy/pathas intended.