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

Reloading the model doesn't work on macOS #12

Closed
hannobraun opened this issue Nov 22, 2021 · 6 comments
Closed

Reloading the model doesn't work on macOS #12

hannobraun opened this issue Nov 22, 2021 · 6 comments
Labels
help wanted Extra attention is needed type: bug Something isn't working

Comments

@hannobraun
Copy link
Owner

I've gotten a report that reloading the model doesn't work on MacOS. I'm just using Notify, so maybe it's an issue in that library.

I don't have a Mac, and no recent experience with MacOS, so any help here would be appreciated. As of this writing, the code for that lives in main.rs, line 56 and following, but it'll probably move somewhere else at some point (likely either model.rs or a dedicated module).

@hannobraun hannobraun added type: bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers topic: model labels Nov 22, 2021
@Bandsberg
Copy link
Contributor

Having tested this on Windows10, the issue is present there aswell.
According to documentation of Notify's AccessKind:

Only some platforms are capable of generating these.

So it makes sense that it does not work for Window10 and MacOS. The documentation does not state which platforms can, and cannot use AccessKind.

A different point relating to a potential fix. I suppose the desired behavior is to recompile on a save event. As I understand Notify documentation currently the recompile is on:

An event describing non-mutating access operations on files

I will look in to a fix using a 'saving of file' event. @hannobraun let me know if I am going in the wrong direction.

FYI. I used debugging CodeLLDB VScode plugin to debug and learned that debugging and the recompiling functionality does not work well together. I believe it is due to the debugger restraining access to the compiled libraries (e.g. xzy.dll) so that they cannot be overwritten. Not an issue for this fix, but an interesting issue nonetheless.

@hannobraun
Copy link
Owner Author

Thanks for looking into this, @Bandsberg. It sounds to me like you're going in the right direction.

I implemented this (on Linux) without looking at the documentation at all. I just printed the events that were triggered and selected the one that seemed to do what I needed, naively assuming that notify behavior was uniform across platforms.

Maybe another event works cross-platform. If not, we can add platform-specific code for this.

FYI. I used debugging CodeLLDB VScode plugin to debug and learned that debugging and the recompiling functionality does not work well together. I believe it is due to the debugger restraining access to the compiled libraries (e.g. xzy.dll) so that they cannot be overwritten. Not an issue for this fix, but an interesting issue nonetheless.

Feel free to open an issue, if you think there's something we can do about this on our end. It should be relatively straight-forward to make the automatic re-compile something that can be enabled/disabled. And once this is possible, we could let the user do this through the UI, or disable it as part of error handling. I don't know, whatever makes sense.

@Bandsberg
Copy link
Contributor

#28 solves the issues of monitoring /watching for changes and recompiling, but model reloading on MacOS still does not work, see PR.

@hannobraun
Copy link
Owner Author

Not sure why this is labeled https://github.com/hannobraun/Fornjot/labels/good%20first%20issue. Removing.

@hannobraun hannobraun removed the good first issue Good for newcomers label Mar 21, 2022
@hannobraun hannobraun changed the title Reloading the model doesn't work on MacOS Reloading the model doesn't work on macOS Apr 11, 2022
@devanlooches
Copy link
Contributor

Tried this out on my mac and it seems to be working fine.

@hannobraun
Copy link
Owner Author

Thank you, @devanlooches! I'm closing this then. If anybody still has problems, please comment here, or open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants