unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.78k stars 270 forks source link

fix off-by-one codebase path (not the one you're thinking of) #1113

Open aryairani opened 4 years ago

aryairani commented 4 years ago

IMO these should provide the same codebase environment:

$ ucm
$ ucm -codebase ~/.unison
$ mv ~/.unison ~/.unison-backup
$ ucm -codebase ~/.unison-backup

but they don't; e.g. the latter will look in ~/.unison-backup/.unison where no codebase exists.

pete-ts commented 4 years ago

I want to go over what the current behaviour is, and what the expected behaviour should/would be.

It feels like the "-codebase" flag, currently at least, gives the impression that the codebase is the directory dir being pointed to, but only as long as the directory contains a ".unison" folder.

After $ mv ~/.unison ~/.unison-backup, a reasonable expectation would be that "the codebase is still under ~". So -codebase ~/.unison-backup should keep working, just like -codebase ~.

Is the expected fix to (for example) start looking for v1/paths/_head, optionally under either dir or dir/.unison, or any other sub-directory?

Which means that the potential outcomes would be:

aryairani commented 4 years ago

Here's what I was thinking: .unison would only show up in the default codebase (~/.unison) and when staging a git push.

ucm would be the same as ucm -codebase ~/.unison. This is not currently the case; currently ucm is the same as ucm -codebase ~

  • When running a transcript, the output of ucm, is to produce a "transcript" folder, that contains .unison.

This is currently what happens. After the PR, ucm would produce a "transcript" folder that contains v1/....

  • The next command suggested by ucm for an interactive run is You can run 'ucm -codebase .../transcript-1c1928baff64f718' to do more work with it

This same command should work as-is, but instead of .../transcript-1c1928baff64f718/.unison/v1/... we would just have .../transcript-1c1928baff64f718/v1/.... If you wanted to mv transcript-1c1928baff64f718 ~/.unison, then that would become your new default codebase.

  • When initialising a new codebase in a target directory (target inclusive of "current dir" .), ucm creates a codebase under "./.unison".

This is how it works currently; after the PR it would create ./v1 and mostly I wouldn't imagine people using mkdir foo; cd foo; ucm -codebase . init they would just use ucm -codebase foo init.

It feels like the "-codebase" flag, currently at least, gives the impression that the codebase is the directory dir being pointed to, but only as long as the directory contains a ".unison" folder.

It gives that impression, but it's specifically looking for .unison/v1/paths/_head. So now, it would just look for v1/paths/_head. See https://github.com/unisonweb/unison/blob/master/parser-typechecker/src/Unison/Codebase/FileCodebase.hs#L114, though this definition would need to be split up, since some of the code assumes it's working relative to v1, some needs to create a .unison, and some assumes it's working relative to the parent of .unison.

Is the expected fix to (for example) start looking for v1/paths/_head, optionally under either dir or dir/.unison, or any other sub-directory?

No, it should just look in one place (dir).

Make sense?

aryairani commented 4 years ago

This would also allow me to load @bascott's codebase (#1512) with just unison flags, and not having to make directories and rename them etc.