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

Utility::FileWatcher watching wrong filepath when using with Utility::Tweakable on Linux #61

Closed
Antoine-dh opened this issue Mar 25, 2019 · 5 comments

Comments

Projects
2 participants
@Antoine-dh
Copy link

commented Mar 25, 2019

When trying to use CORRADE_TWEAKABLE() with Utility::Tweakable I get a runtime error message from Utility::FileWatcher on Linux

Utility::FileWatcher: can't stat home/ad/CLionProjects/MagnumProject/src/MyApplication.cpp: No such file or directory, aborting watch
Utility::Tweakable: watching for changes in home/ad/CLionProjects/MagnumProject/src/MyApplication.cpp

I believe the root / was not inserted at the begin of the filepath string causing the filepath to be relative instead of absolute and the file not to be found.

@mosra

This comment has been minimized.

Copy link
Owner

commented Mar 26, 2019

Hi! The path is gathered from the __FILE__ preprocessor macro, so if your buildsystem underneath does something like

cd /
g++ home/ad/CLionProjects/MagnumProject/src/MyApplication.cpp -o MyApplication

then you'll get the path without the initial slash in __FILE__. I suspect this is something CLion does. As another example, in my case, when using ninja, the paths are relative to the build directory, so instead I'd get ../src/MyApplication.cpp in __FILE__ (which is nice and short). OTOH with plain make, as far as I remember, the paths were always full and absolute.

There's the Tweakable::enable(const std::string&, const std::string&) overload that allows you to set the path prefix explicitly, it's there to cover exactly these cases. From the top of my head, I think calling

tweakable.enable("", "/");

could do the trick. Hope this helps! :)

@mosra mosra added this to TODO in Utility via automation Mar 26, 2019

@Antoine-dh

This comment has been minimized.

Copy link
Author

commented Mar 27, 2019

Hello, thanks for your answer.
tweakable.enable("", "/"); actually did the trick
but I still think there is an issue on your side as printing __FILE__ using the same build system gives me the correct path with the root slash

@mosra

This comment has been minimized.

Copy link
Owner

commented Mar 28, 2019

Oh, okay, thanks for the heads-up! 👍 Looking at the code, I think I know where the problem is (and I'm sorry about the conspiration theories above). This is what I get for testing just with Ninja, which has always relative paths 😅

Is it possible for you to try the following patch (and running with just the argument-less enable()) and tell me if it works correctly in your case?

diff --git a/src/Corrade/Utility/Tweakable.cpp b/src/Corrade/Utility/Tweakable.cpp
index 8e0f8b7c..f51d847d 100644
--- a/src/Corrade/Utility/Tweakable.cpp
+++ b/src/Corrade/Utility/Tweakable.cpp
@@ -110,7 +110,7 @@ std::pair<bool, void*> Tweakable::registerVariable(const char* const file, const
            works correctly. */
         /** @todo maybe some Directory utility for this? */
         std::string stripped = String::stripPrefix(Directory::fromNativeSeparators(file), _data->prefix);
-        if(!stripped.empty() && stripped.front() == '/')
+        if(!_data->prefix.empty() && !stripped.empty() && stripped.front() == '/')
             stripped.erase(0, 1);
 
         const std::string watchPath = Directory::join(_data->replace, stripped);

Thanks a lot!

@Antoine-dh

This comment has been minimized.

Copy link
Author

commented Mar 28, 2019

I applied your patch and it did fix the initial issue, thank you

@mosra

This comment has been minimized.

Copy link
Owner

commented Mar 29, 2019

Fixed on master: edeeedb. Thanks for the report and testing! 👍

@mosra mosra closed this Mar 29, 2019

Utility automation moved this from TODO to Done Mar 29, 2019

@mosra mosra added this to the 2019.0b milestone Mar 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.