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

writing to the filesystem during DrRacket start up #79

Closed
rfindler opened this issue Dec 11, 2023 · 13 comments
Closed

writing to the filesystem during DrRacket start up #79

rfindler opened this issue Dec 11, 2023 · 13 comments

Comments

@rfindler
Copy link
Collaborator

I'm seeing new errors in DrRacket when running in drdr. Here's an example.

This test is ensuring that DrRacket does not write to the filesystem when starting up, but the (new?) strategy that quickscript is using seems to write to the disk during startup.

I'm thinking that it is probably still a good idea to avoid writing to the filesystem during start up. If the directory creation is delayed until someone does an action with quickscript, then we can get an error message to the user in a context when DrRacket is all started up. But if it happens during DrRacket's start up, we might not even be able to start up DrRacket.

@Metaxal
Copy link
Owner

Metaxal commented Dec 12, 2023

ah, sorry, I forgot about that. Let me push a repair — as soon as I have setup my laptop again.

@Metaxal
Copy link
Owner

Metaxal commented Dec 12, 2023

After thinking about it, i'm unsure how to solve this correctly.

The recent change is that the quickscript package copies a few default scripts in the user-specific user-scripts directory, which belongs to a subdirectory of racket's config directory. This directory does not exist yet after a first-time installation of Racket.

Thus, the action (ensure-user-script-dir-exists!) must be done before the user calls the Script menu.
Unfortunately, even placing (ensure-user-script-dir-exists!) in the on-demand callback of the Scripts menu doesn't pass the no-leak test :/

Any idea?

cc @spdegabrielle

@rfindler
Copy link
Collaborator Author

How important is it, specifically, to populate the user's directory immediately?

There is a similar issue with the preferences (which, I believe, is what motivated this test case) where I make sure that no preferences are set during start up which, in turn, ensures that the preferences file isn't written to (and, if it doesn't exist, it won't be created).

Could things be delayed until some action is taken in the GUI (which would happen after DrRacket starts up and thus, if the filesystem had gone wrong, they'd get an error and possibly be able to still use DrRacket otherwise)?

@Metaxal
Copy link
Owner

Metaxal commented Dec 13, 2023

I think I'm somewhat confused. Is the purpose of this test case that drracket should work even if it's not possible to write to the filesystem? I'm not really sure why this is important — is an IDE really useful if it can't write to the filesystem?

Here's the use-case I'm expecting:
A fresh user installs racket+drracket, starts drracket, and starts doing some work. They go through the menus, look at the Scripts menu. They were told that Quickscript comes with a few pre-installed scripts — great, that's user-friendly. But... where are they? The Scripts menu only shows this Manage Scripts submenu, which looks a bit technical for now.

So when should the user expect these "pre-loaded scripts" to show up?

My idea was to copy these preloaded scripts into the user-scripts subdirectory of the user config directory, so that they can start trying them out and hacking them right away. But this can't even happen on the demand-callback of the Scripts menu, so necessarily it won't show up the first time the user clicks on this menu.

Note that even if @LiberalArtist 's proposal #73 works out for quickscript packages, the installed scripts won't be in the user-scripts directory and are not meant to be modified directly.

One (poor) solution would be to have a "Generate some basic scripts" menu item in the Manage Scripts submenu, but this menu is already large enough (with even some duplicate items to answer a request from Matthias), and frankly that's not really user-friendly. If we go this way, I'd rather have a more complete solution where the user can choose between many scripts from a list, to download from a particular directory somewhere on the internet and copy them in the user-scripts directory. Like url2script but where a list of scripts urls could be obtained. I guess I would like this approach, but I'm also rather short on free time, and it also seems to be asking for trouble and lots of bug fixes — if it's part of the main distribution.

That said, if that's too much trouble I'll just remove these preloaded scripts.

@rfindler
Copy link
Collaborator Author

Hi @Metaxal --- of course, IDEs need to write to the filesystem. That's not the issue. The issue is specifically that writing to the filesystem during start up introduces an extra source of difficulty when trying to ensure that the app gets everything loaded into place properly and it is functioning. For example, if Quickscript were to find out that the user's dotfile directory weren't writable (some people might just do that as a matter of course for various apps, maybe?) then we would still want DrRacket to start up and to be loaded properly.

Of course, DrRacket is already catching exceptions during the start up of tools and disabling them but this isn't foolproof. For example, if quiscript were to install some of its mixins and then write to the filesystem and then install more mixins, DrRacket might end up in a state where some mixins were installed but others weren't and so things are then likely to break in strange ways after that.

In contrast to all that, once DrRacket is started up, everything happens on GUI callbacks and any exception that gets raised only terminates that one callback. That's not to say that there aren't still important problems that can happen and ways that things can go wrong (and have, in previous versions of DrRacket), but the situation is a lot simpler and often the user still has a functioning DrRacket if they just don't do that one opertation that triggers the filesystem exception. They might even be able to read the exception error message and adjust something somewhere to make that operation start working but this is unlikely to be a possibility if the exception happens during startup. And the user may just be, say, interested in getting their homework done, not interested in quickscripts (at that moment). We shouldn't make them fix whatever the problem is that keeps quickscript from creating this directory before they are allowed to start their homework.

Fundamentally, working with the filesystem introduces the problem of hard-to-predict exceptions get thrown. It seems to me like a good design choice to avoid such errors completely during DrRacket's start up.

Does this argument make more sense now?


As for built-in scripts, why do the builtin scripts have to be copied out of quickscript into another place on the filesystem during start up of DrRacket? Could there instead be a means where a user decides they want to start from an existing script and modify it and then quickscript could, at that moment, make a copy of the script into the user's directory (creating it if necessary)?

Or maybe there is another, more creative, way to approach this problem? I'm happy to help figure out something that works better if at all possible.

And, if not, I can just disable quickscript specifically during this test, effectively allowing it to write to the filesystem during start up. We'll just have to be careful that quickscript correctly protects itself (and the rest of DrRacket) from crashing or getting into a bad state if something goes wrong!

@Metaxal
Copy link
Owner

Metaxal commented Dec 13, 2023

Does this argument make more sense now?

I think so. Is it fair to reduce your explanation to: Letting plugins write to the filesystem on startup substantially increases the likelihood of something to go wrong and leave the user with a DrRacket in a bad state, or unusable.

why do the builtin scripts have to be copied out of quickscript into another place on the filesystem during start up of DrRacket?

So that there are already a couple of scripts in the menu, so that a new user can try them out, then edit them and play with the code. Asking the user to click on a button before these default scripts appear in the menu would kind of defeat the purpose.

I don't like the idea of making an exception for QS at the drr level too much. Sounds like asking for trouble.
Possibilities:

  1. QS attempts to write to the filesystem to create the user-scripts directory. If that fails with an exception, QS catches the exception and just skips. Since that would still trigger the sandbox, maybe we could check beforehand whether writing to the filesystem is allowed at all. This is like pushing the exception idea within QS, but it also feels like cheating, kind of.
  2. Including default scripts would certainly be more user-friendly, but is also not a necessity per se. So we could just not include them at all, as we did in the past.

@rfindler
Copy link
Collaborator Author

What do you think about having the scripts all there in the menu from the start (because quickscript looks for them inside its own directories) but then as soon as the user opens one of them for editing (via the quickscript menus), quickscript either offers to copy them to the user's directory or just does it automatically and then opens the script there?

I guess with the latter there is the problem that multiple scripts with the same name (and same behavior?) appear in the menu so we'd want to think about how that can happen and how to avoid it.

@Metaxal
Copy link
Owner

Metaxal commented Dec 13, 2023

Refining: Is there a way to have an actual on-demand-callback that is triggered only when the user clicks on the menu? And actually, why is it triggered at all during the no-leak test? (does this test click on all buttons?)

@Metaxal
Copy link
Owner

Metaxal commented Dec 13, 2023

in-between these two solutions is to wait for @LiberalArtist 's fix of the register business, in which case QS can come bundled with some scripts, but that won't allow for hacking them straight away.

This would have the advantage of keeping things simple and avoiding exceptions.
@spdegabrielle Would this work for you?

@rfindler
Copy link
Collaborator Author

Refining: Is there a way to have an actual on-demand-callback that is triggered only when the user clicks on the menu? And actually, why is it triggered at all during the no-leak test? (does this test click on all buttons?)

I am not sure but I suspect it is because the on-demand callback's semantics only require that it is called, not that it isn't called. But we can work around this, I feel pretty sure. We can just have other callbacks that are a bit more well defined if we need them.

@Metaxal
Copy link
Owner

Metaxal commented Dec 13, 2023

I am not sure but I suspect it is because the on-demand callback's semantics only require that it is called, not that it isn't called. But we can work around this, I feel pretty sure. We can just have other callbacks that are a bit more well defined if we need them.

Ok, thanks for considering the idea, though that sounds like significant additional work for this very small feature.

@rfindler
Copy link
Collaborator Author

I'm open to doing extra work to get what we want and (IMO, the big one) try to keep DrRacket as reliable and friendly to work with as possible. Implementing an extra callback to handle the creation/population of this directory seems not too difficult (although defining when it is supposed to happen might be a bit more complex, I'm not sure).

@Metaxal
Copy link
Owner

Metaxal commented Dec 14, 2023

Thanks for the offer, Robby. For now I've removed the scripts so that DrR is in a good state while we think of a proper solution. Though it may be a good idea to think in the broader context of #73

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

No branches or pull requests

2 participants