Allow packages to register scripts declaratively

[LiberalArtist]

This is incomplete and untested (I haven't even looked at updating the test suite yet), but here is a draft of my attempt at resolving #73. I'll comment with some more thoughts tomorrow.

[LiberalArtist]

I suspect it might be better for (compile-library) not to attempt to recompile scripts from collections, but maybe it's harmless?

[Metaxal]

Matthew recently had some issues with this actually (personal email from Dec 15, 2023, cc robby): when working on the GUI, launching DrRacket was getting slow because some script compilation triggered compilation of the GUI
@rfindler said he'd think about it. Discussion still ongoing.

[LiberalArtist]

This special-case for info.rkt feels a bit unsatisfying, but it's definitely the easiest approach.

[Metaxal]

Then we should also prevent the user from creating a script named info on the gui:
https://github.com/Metaxal/quickscript/blob/master/tool.rkt#L127

[LiberalArtist]

Good catch!

[LiberalArtist]

I've updated (new-script), though I'm a bit dubious of its overall approach.

[LiberalArtist]

Should we initialize the preference with an existing library.rktd file if one exists? One reason not to do so is that we would carry forward the duplication of existing directories as in Metaxal/quickscript-extra#27, which this change would otherwise fix.

[Metaxal]

The more I think about it, the more I think the proper and simpler fix looks like this:

• For installed collections, scripts are added automatically and only exclusions (either directories or single scripts) are stored. This avoids both the duplication issue and the register-on-setup problem.
• For paths added manually by the user, store the included paths and the excluded scripts (as is currently the case)

This can be done with only little change the library system, independently of the preferences system.

[LiberalArtist]

I think this comment is what this PR currently does, except that there is no mechanism for excluding whole directories. That's a feature we could consider, but it raises more questions, and it seemed to me that either uninstalling the unwanted package or excluding all of the scripts individually would be good enough for most cases.

It's true that changing from library.rktd to the preferences system isn't directly related to the rest of this change. I don't think we can just continue using library.rktd, though, because it needs to be able to be understood by old versions of Racket, and we want to store at least some information that old versions can't handle. We could start using library- v2.rktd or something, but this compatibility problem is exactly the same as applies to the rest of the preferences system, which is why I suggested adopting that already-existing solution.

What I was trying to ask here is whether, if new-style data hasn't been saved (regardless of how we decide to save it), we should try to import an old library.rktd file if one exists. The more I think about it, the more strongly I think not. There seem to be three categories of entries that might be present in library.rktd:

1. user-script-dir, which we will add by default anyway. We could import any exclusions from user-script-dir if that seemed worthwhile.
2. Collection directories from an old Racket version, perhaps including duplicates due to Auto-update only adds to library path quickscript-extra#27. Importing these would be a bug! As long as the corresponding package is installed, the new mechanism will take care of these. Unfortunately I don't see any good way to identify which entries these are, which means I don't know how we could import only the exclusions, if any.
3. Non-collection directories explicitly added by the user (i.e. not user-script-dir) are what would be nicest to be able to import, but we have no way of distinguishing these from the collection-based directories that should not be imported.

I think this means we have the choice of two kinds of less-than-ideal behavior: either we import old collection/package directories that we shouldn't; or we omit explicitly added directory entries that we should. I think it's better to avoid the wrong imports. I expect that more people have gotten entries in library.rktd from this package in 8.11 or quickscript-extra at any version than have explicitly added directories other than user-scripts-dir. Furthermore, anyone who has explicitly added directories has used the Quickscript library manager to do so, and can be asked to do so again, whereas someone who only got scripts from packages may never have used the Quickscript library manager.

[Metaxal]

This is a one-off, so as long as this breaking change is well advertized I'm mostly okay with letting the user reconfigure their library.

If that's not too much effort we could still consider item 1. though, which should be the most common one.
It would be nice to treat exclusions from quickscript-extra and quickscript-competition-2020 specifically, if that's not too much trouble, because of shadow scripts — otherwise this would lead to duplicate entries in the menu. But again, I don't feel too strongly on this, and it will depend on the required effort.

[LiberalArtist]

I've pushed further commits to import old exclusions for user-script-dir by default.

I gave some thought to heuristics to find other exclusions we'd want to import, but, the more I think about it, the more strongly I think the right thing to do is to provide documentation for the user. Even just considering exclusions from quickscript-extra due to shadow scripts, even assuming we could reliably recognize it and ignoring Metaxal/quickscript-extra#27, importing the exclusion wouldn't be enough to fully solve the problem: the user would also need to edit the shadow script itself, because the old script would use something like:

(require (file "/absolute/path/to/THE/OLD/VERSION/OF/quickscript-extra/scripts/tweet.rkt"))

(This PR fixes that going forward by using collection-based module paths when shadowing collection-based scripts.)

[Metaxal]

Thanks for all this!

It doesn't seem like there is a seamless solution for the user unfortunately, as they will be left with old shadow scripts to delete or adapt and disable the corresponding original scripts, or new shadow scripts to rewrite.

But the previous state with duplicate registered directories was bad either way. So your approach is the best I can think of for now. I'm just worried that some users will miss the announcement about what to do. Maybe we should include a section in the docs about pre-to-post Racket 8.13.

[rfindler]

Matthew recently had some issues with this actually (personal email from Dec 15, 2023, cc robby): when working on the GUI, launching DrRacket was getting slow because some script compilation triggered compilation of the GUI
@rfindler said he'd think about it. Discussion still ongoing.

My thoughts didn't turn up much, unfortunately. The suggestion Matthew had is that quickscript should use the manager-skip-file-handler as drracket already does, which seems like a good idea to me.

In that thread, @Metaxal suggested to stop auto-compiling scripts, unless the scripts are outside the package system (this would probably also require the change in the previous paragraph). That seems like a good idea to me, especially if we're moving to a situation where, by default, there are no scripts outside the package system.

[Metaxal]

Another simple idea was to actually delete the .zo files for user-based scripts, and remove the compilation part altogether.

[rfindler]

Another simple idea was to actually delete the .zo files for user-based scripts, and remove the compilation part altogether.

Yes, that might be a good idea. Then the script files become like other files in racket and people can expect to manage the compilation of them on their own.

[LiberalArtist]

Re #73 (comment):

• I'm okay with using the preference system. I'm just worried that the prefs file is getting big and any change of any pref requires rewriting the whole file, which becomes slower and slower as we add more prefs. Or can we use a separate quickscripts-prefs.rktd file maybe?

Looking at my own racket-prefs.rktd, existing prefs like readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos seem to store much more data than I'd expect for my proposed plt:quickscript:library, which only holds directories and exclusions explicitly added by the user. I'd defer to @rfindler on the proper use of the preferences system, though.

It does seem possible to use a separate quickscripts-prefs.rktd file, and it would still let us rely on support from the preferences system for locking, compatibility across versions, etc., but it seemed better to avoid the complexity of a separate file unless it is actually beneficial.
`

* I'm okay with keeping things consistent with `test-include-paths` and friends, except that `all` doesn't seem adequate for quickscripts (`info.rkt` is not a script for example), and it seems to me that the default should be a `scripts` subdirectory instead — or automatically skipping files like `info.rkt` but that seems fragile.

I could also imagine (define quickscript-directory #t) and document that values other than #t are reserved for future use.

Then maybe just (define quickscript-directory "scripts") instead?

I ended up going with quickscript-directory instead of imitating test-include-paths et al. for two main reasons:

1. The existing UI and other parts of the design have a model that script directories contain script files (as immediate children, with constrained file names), which may be included or included. Keeping that model helped to minimize the disruption from these changes. In particular, that meant I wanted the info.rkt entries to only specify directories. That's different from entries like test-include-paths, which can specify individual files and, when they specify a directory, apply to the directoy's recursive contents instead of only its immediate contents.
2. I'd hoped there would be a library function to handle matching test-include-paths-style specifications, but there doesn't seem to be. (Some of the examples, like compile-omit-paths, have to work under extra restrictions.) The flexibility those specifications allow seems to come with some pitfalls, like the potential for "../../../../etc/passwd", which I'm not sure even the current uses handle.

I can see how (define quickscripts-directory "scripts") would allow a package like quickscript-extra to avoid having to add an additional info.rkt file, and I think it would be viable to allow a value of a single collection-name-element?.

I like that the current (define quickscript-directory #t) would allow you to create a package of only QS scripts by adding just an info.rkt file, without having to move the scripts to a subdirectory. (Github Gists do not allow subdirectories, for example.) A side benefit is that, if we accept the restriction that (script-file? "info.rkt") is #false, putting the info.rkt file in the script directory prevents accidentally attempting to create a script with that name.

One issue though: quickscript-test comes with a bunch a scripts that should loaded only when running the test suite. So these scripts must be excluded by default. How could this happen? (Currently, these scripts are not included by default, and registered only for the duration of the tests.)

I hadn't realized https://github.com/Metaxal/quickscript-test existed! I haven't looked at it yet, but I expect it should not use the info.rkt mechanism and instead only add its script directory when setting up to run tests (and remove it on teardown, or else not save it).

It want to look into how other things based on find-relevant-directories handle testing.

The new mechanism also requires the creation of a collection to be able to add a directory with scripts. It's heavier than the un/register approach and can't be done (easily) directly within the library GUI I suspect.

I suppose we could have both: (i) explicit exclusions of find-relevant-directories and (ii) explicit inclusions of user-added paths.

(i) would be for collections and (ii) would be for non-collections.

Thoughts?

I think this PR already does what you propose. It is still possible to use the GUI to add a new non-collection directory just as before.

* Do keep in mind how to create shadow scripts from existing collection-based scripts. (I don't think I have a test for this though, since it's a little tricky as it depends on adding ad-hoc collections and stuff. Not a good excuse anyhow.)

* The library should still show all the scripts that are loaded or excluded.

There is a bug with shadowing collection-based scripts currently, but these are both supposed to work.

[rfindler]

• I'm okay with using the preference system. I'm just worried that the prefs file is getting big and any change of any pref requires rewriting the whole file, which becomes slower and slower as we add more prefs. Or can we use a separate quickscripts-prefs.rktd file maybe?

Looking at my own racket-prefs.rktd, existing prefs like readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos seem to store much more data than I'd expect for my proposed plt:quickscript:library, which only holds directories and exclusions explicitly added by the user. I'd defer to @rfindler on the proper use of the preferences system, though.

We have had some performance troubles in the past with preferences getting too large in certain situations and there's even some support built into DrRacket so we can get information from users to debug this kind of problem. Wave your mouse just to the left of the memory in the bottom right of the DrRacket window, where this arrow is pointing:

and you should see an ugly "P" appear. Click on it to get some information about the preferences.

I don't think we're in the "use a different mechanism" territory here. I'd say that the current situation is probably fine and, absent performance problems, saving the preferences to the disk on write is preferable. But there could be some improvements and they'd be welcome if someone has the energy.

[Metaxal]

@LiberalArtist All fine with me.

Regarding the single preference file, personally I find putting everything in there somewhat jarring, efficiency-wise and conceptually — but I can certainly understand historical reasons and organic development. In particular the interaction history should be in its own separate ad-hoc pref file, as well as recently-opened files, and any other sub-preferences that can take an arbitrary amount of space. Like the QS library.

Splitting in files also allows for the user to throw away only one file when something goes wrong (say, getting rid of the history, or throwing away the QS preferences), without having to carefully edit a large data file with everything it.
I'm not asking to change the existing preferences here, but I think if we can keep a separate preferences file for QS, that would be better.

[LiberalArtist]

Re @Metaxal:

Regarding the single preference file, personally I find putting everything in there somewhat jarring, efficiency-wise and conceptually — but I can certainly understand historical reasons and organic development. In particular the interaction history should be in its own separate ad-hoc pref file, as well as recently-opened files, and any other sub-preferences that can take an arbitrary amount of space. Like the QS library.

Splitting in files also allows for the user to throw away only one file when something goes wrong (say, getting rid of the history, or throwing away the QS preferences), without having to carefully edit a large data file with everything it. I'm not asking to change the existing preferences here, but I think if we can keep a separate preferences file for QS, that would be better.

In LiberalArtist@87b4630 I've tried using a separate quickscript-prefs.rktd file. I don't like it, though.

With the benefit of hindsight, I would also prefer for readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos not to be stored in (find-system-path 'pref-file) or even (find-system-path 'pref-dir): I like the organization of the XDG Base Directory Specification, which puts such things in XDG_STATE_HOME, as opposed to XDG_CONFIG_HOME or XDG_DATA_HOME. (Even there, this distinction was only added in 2021!)

The Quickscript library, though, seems to me like it really is a user configuration preference. It is also much smaller than the large "preferences" I listed, in part because it stores only exclusions and non-collection directories. (Handling user-script-dir specially per #81 (comment) would let it be even smaller.) Concretely, my racket-prefs.rktd file is 31,688 bytes, while, with two shadow scripts, my stand-alone quickscript-prefs.rktd file is the following 528 bytes:

(
 (plt:framework-pref:plt:quickscript:library
  (
   (3)
   2
   ((#"/home/philip/code/racket/pkgs/quickscript/private/library.rkt" . deserialize-info:library-data-v0) ((lib "racket/private/set-types.rkt") . deserialize-info:immutable-custom-set-v0))
   0
   ()
   ()
   (0 (h - (equal-always) ((p+ #"/home/philip/.config/racket/quickscript/user-scripts/" . unix) 1 #f (h - (equal-always)))) (1 #f (h - (equal-always) ((q lib "quickscript/scripts/eyes.rkt") . #t) ((q lib "quickscript-extra/scripts/tweet.rkt") . #t))))
  ))
)

While parameterizing preferences:low-level-get-preference and preferences:low-level-put-preferences works in LiberalArtist@87b4630, it seems like it would complicate making deeper use of the preference system in the future (including the debugging info @rfindler mentioned in #81 (comment)). More immediately, it seems like it may require adding more custom hooks for testing, since any attempt to override those parameters for testing would now be overridden by our parameterization.

All of that said, I think even using a separate quickscript-prefs.rktd seems better than managing all the file IO, caching, and failure issues manually.

[Metaxal]

You have a point that exclusion should be more sparse than inclusion — except if someone makes extensive use of shadow scripts, which is not unlikely.

I agree that using the preference system is better than a custom file. IIUC however, it seems that the preference system is not currently well suited to have multiple preference files, is that what you're saying?

Assume we choose to use the common racket-prefs.rktd file for QS. How much hassle would it be to later switch to a custom quickscript-prefs.rktd versus switching now? If it's at least the same effort, then let's use racket-prefs.rktd for now. Otherwise let's assess. (@rfindler may have a more informed opinion on this matter.)

It still feels pretty wrong to me that a plugin would use the common racket-prefs.rktd 😛

[rfindler]

If it's at least the same effort, then let's use racket-prefs.rktd for now. Otherwise let's assess. (@rfindler may have a more informed opinion on this matter.)

I think using the framework preferences system is a good choice. It does a lot of things for you that are easy to get wrong. I wouldn't worry about "racket-lang.org" appearing in the filename. Check Syntax is a plugin, after all.

That said, I do think that @LiberalArtist makes a good point that it might be beneficial to move more "state"-like preferences into their own place. It seems better for the usecase of people actually editing the preferences file. There may be other reasons to want to do it too.

[Metaxal]

How is the progress going @LiberalArtist ? Any blocker?

[LiberalArtist]

How is the progress going @LiberalArtist ? Any blocker?

I'm expecting to have time to get back to this in the next few weeks. Having missed 8.13, I'd like to get this done soon so we have some real-world experience before 8.14. I'll have to page the context back in, but IIRC it was fairly close: I think I was working on the test suite.

[Metaxal]

Thanks for the update!

…

On Sat, Apr 27, 2024 at 2:18 AM Philip McGrath ***@***.***> wrote: How is the progress going @LiberalArtist <https://github.com/LiberalArtist> ? Any blocker? I'm expecting to have time to get back to this in the next few weeks. Having missed 8.13, I'd like to get this done soon so we have some real-world experience before 8.14. I'll have to page the context back in, but IIRC it was fairly close: I think I was working on the test suite. — Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMU2HHK7WWRUVLTE4CFZH3Y7L4EZAVCNFSM6AAAAABBQECFEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBQGI4TQMBVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Metaxal]

Hi @LiberalArtist , can you give me an update on this? Thanks!