medley
medley copied to clipboard
Interlisp
Medley interface for PR branch comparisons and approvals
This issue addresses the question whether and how Medley source-file (and Tedit-file) comparison tools can be used to make it easier to compare a remote branch for which a pull request has been issued with the origin/master branch (or whatever branch) is the targete of the eventual merge. This has nothing to do with branches (even the master) that may or may not already be part of the local clone.
Tentatively, here is my understanding of the work process:
You get an email with a request-to-review, and a #xxx link to the PR. The email doesn't tell you the branch, so you click on the link, and there you find the branch name (bbb) that you can remember or copy to the clipboard.
Inside Medley (these functions execute git shell commands of one sort or another)
(GIT-ADD-WORKTREE 'origin/bbb) whether or not you already have a version in your local clone. You don't want this process to mess up your current clone, switch branches, etc. The branch bbb goes into the local {UNIX}.../worktrees/bbb/
(GIT-ADD-WORKTREE 'origin/master) again, without messing up the files of your current clone: {UNIX}.../worktrees/master/
(GIT-BRANCH-DIFF 'worktrees/bbb 'worktrees/master) to find out which files git thinks are different.
For each of those files: (COMPARESOURCES 'worktrees/bbb/file 'worktrees/master/file). (Here is where we still need some tuning to make it easier to poke around and see the differences (clickable imageobjects in Tedit, side-by-side SEDIT)
When all done
(GIT-REMOVE-WORKTREE 'bbb) (GIT-REMOVE-WORKTREE 'master)
This can all be wrapped up in a little Medley EXEC command: prc bbb (pull request compare) that will do the comparison on each changed file.
If the merge looks good, go back to github to click "approve".
On further thought, maybe the whole worktree isn't needed. If the diff works on the origin/bbb and origin/master to give just a list of changed files, then is there a git command that will checkout a file from a branch to an arbitrary local path?
That would be enough to do the approval comparisons, without all the extra overhead of the worktree.
The worktree checkout could be used for another action, namely, to build and actually test in the branches, again, without messing up the main repository-tracking directories.
I have the basic set up implemented, playing with it on one of Larry's PRs. We need a window/region manager for correlated files to organized the interaction with the comparison displays. Also, although we have COMPARESOURCES for lisp files, and COMPARETEXT for text files, I don't think we have a comparer for Tedit files.
There's a GitHub network api for people who are developing other applications. https://docs.github.com/en/rest There is also a GitHub developer program -- https://docs.github.com/en/developers/overview/github-developer-program
@rmkaplan ... is there a git command that will checkout a file from a branch to an arbitrary local path?
The TLDR: You can always switch to a branch locally, copy the interesting file to a non-repo-working location and then switch back to main or whatever for making a comparison. Could probably automate that locally.
Just-in-case details:
This is maybe just a nomenclature matter. Just in case, I want to point out that there is not exactly any such thing as a file-level checkout in git. Essentially, the computer has a working folder that is a clone (if not the original) of a Git-managed repository. The .git directory at the top level of the working folder is in fact a hook into the (local) repository. It has usually all of the history of the project from the beginning of the repo creation.
Usually, the working folder is synchronized with the local Git main or master branch. When you change branches using Git (new branch or existing one), the working folder changes in alignment. No matter the purpose of the branch, it is the whole repo that the branch was from at some point, with the branch changes anywhere in that.
If you mess up a file in the current working folder, you can always use Git to restore it from the local repository, enough reason to use Git even if you don't have an origin repo in the cloud somewhere (i.e., GitHub).
Sorry for explaining something you probably already know better than I.
PS: If this is about pull requests, it helps to have a clone of the repository (and branch) the request is from. Maybe?
@rmkaplan I am looking at the man page for the git diff operation and it should do what you want except I have no comprehension of the interaction/interdependence with medley :(
On further thought, maybe the whole worktree isn't needed. If the diff works on the origin/bbb and origin/master to give just a list of changed files, then is there a git command that will checkout a file from a branch to an arbitrary local path?
You can get the list of changed files between origin/master and origin/bbb from:
git diff --name-only origin/master..origin/bbb
You can't "check out" the file from a branch, but you can get it to standard output which you can redirect into an arbitrary file... If the git diff above shows you that, for example, lispusers/FOO has changed then you can do
git show origin/master:lispusers/FOO >/tmp/FOO-master
git show origin/bbb:lispusers/FOO >/tmp/FOO-bbb
and then do your (Medley) comparison between /tmp/FOO-master and /tmp/FOO-bbb (and then clean them up).
If you're executing the git show from Medley you could be collecting the output from the stream into a {core} files rather than relying on the Unix file system underneath.
That looks very good, presuming that the show is byte-wise and complete. Putting it in {core} means that the files will vanish without a trace when Medley goes away. Or {nodircore}, and they will get collected as soon as the comparison windows close down.
On Nov 20, 2021, at 2:52 PM, Nick Briggs @.***> wrote:
On further thought, maybe the whole worktree isn't needed. If the diff works on the origin/bbb and origin/master to give just a list of changed files, then is there a git command that will checkout a file from a branch to an arbitrary local path?
You can get the list of changed files between origin/master and origin/bbb from:
git diff --name-only origin/master..origin/bbb You can't "check out" the file from a branch, but you can get it to standard output which you can redirect into an arbitrary file... If the git diff above shows you that, for example, lispusers/FOO has changed then you can do
git show origin/master:lispusers/FOO >/tmp/FOO-master git show origin/bbb:lispusers/FOO >/tmp/FOO-bbb and then do your (Medley) comparison between /tmp/FOO-master and /tmp/FOO-bbb (and then clean them up).
If you're executing the git show from Medley you could be collecting the output from the stream into a {core} files rather than relying on the Unix file system underneath.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974722713, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJOM7MZVCY5RGC5YDQDUNARDXANCNFSM5IM4ORTQ.
Actually, ShellCommand on UNIXPRINT, may not always be transparent in this way. It is doing (PRINTCCODE (READCCODE processstream) medleystream). Should it be doing (\BOUT (\BIN)) instead?
On Nov 20, 2021, at 3:55 PM, Nick Briggs @.***> wrote:
presuming that the show is byte-wise and complete
It is supposed to be.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974728445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJPRCXGL4A623G6VZNDUNAYNZANCNFSM5IM4ORTQ.
isn't the shell is UTF-8 and (for now) the medley stream is XCCS. It _should be transcoding.
I see that currently the process stream doesn’t actually have a specified external format, so it is probably defaulting to XCCS behavior. Perhaps ShellCommand should set it as UTF-8 after it is created, and the normal mode is that it traffics in characters.
But there is still the question of whether there is a raw-byte level for the process stream interface.
On Nov 20, 2021, at 8:11 PM, Larry Masinter @.***> wrote:
isn't the shell is UTF-8 and (for now) the medley stream is XCCS. It _should be transcoding.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974752521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJK3GT5WNRIMJNL4RVLUNBWPDANCNFSM5IM4ORTQ.
The exact form of output would depend on the LANG environment variable. I think we can assume that it's going to be UTF-8 compatible (could be ASCII subset). Maiko is just going to read the bytes and pass them up to Lisp -- it neither knows nor cares what they actually represent, but Lisp should probably know that they're UTF-8, not XCCS!
Suppose the ShellCommand is just cat of a file, and it happens to be an XCCS-encoded Tedit file, or a binary lcom. I don’t think the ShellCommand should be doing character conversion if it is being used to get a binary file.
For this particular git-show scenario, maybe it would be better to use the > tmp/… so that it doesn’t rely on the process stream to provide the file contents? Will that preserve all of the bytes?
On Nov 20, 2021, at 11:08 PM, Nick Briggs @.***> wrote:
The exact form of output would depend on the LANG environment variable. I think we can assume that it's going to be UTF-8 compatible (could be ASCII subset). Maiko is just going to read the bytes and pass them up to Lisp -- it neither knows nor cares what they actually represent, but Lisp should probably know that they're UTF-8, not XCCS!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974766588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJKGLG5DH3DECFXWR2DUNCLGXANCNFSM5IM4ORTQ.
Suppose the ShellCommand is just cat of a file, and it happens to be an XCCS-encoded Tedit file, or a binary lcom. I don’t think the ShellCommand should be doing character conversion if it is being used to get a binary file.
Yeah, you have a point there. The underlying stream (from CREATE-PROCESS-STREAM) is just bytes -- and ShellCommand is (currently) doing the next-level interpretation via READCCODE. I don't know that there's a reason to write a tmp file and then read it vs. calling CREATE-PROCESS-STREAM and reading it as you would have the tmp file.
OK, I’ll continue with the process stream, for this purpose. (This is needed even for Lisp source files, if their define-file-info says that their format is not XCCS.)
It is a separate issue whether ShellCommand should use bin’s and bouts instead of the character functions for its default behavior. However, if it remains as a character-level interface, then we should assert that the stream is UTF8, as Larry points out. But that's only possible if Unicode is in the loadup. I.e., it won’t currently work for the Lisp sysout, only Full.
This probably should be done at the lower CREATE-PROCESS-STREAM level. It already asserts that the stream's EOL convention is LF. I propose to extend it with a condition that its external format is UTF8 if that format exists. (Perhaps then it would no longer be necessary for clipboard to also assert UTF8, leave it up to the process stream)
On Nov 21, 2021, at 8:26 AM, Nick Briggs @.***> wrote:
Suppose the ShellCommand is just cat of a file, and it happens to be an XCCS-encoded Tedit file, or a binary lcom. I don’t think the ShellCommand should be doing character conversion if it is being used to get a binary file.
Yeah, you have a point there. The underlying stream (from CREATE-PROCESS-STREAM) is just bytes -- and ShellCommand is (currently) doing the next-level interpretation via READCCODE. I don't know that there's a reason to write a tmp file and then read it vs. calling CREATE-PROCESS-STREAM and reading it as you would have the tmp file.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974849187, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJMSKXBZ4CQXHZZIEZLUNEMUZANCNFSM5IM4ORTQ.
back to the workflow that started this issue (the ShellCommand character handling should be a separate issue; ShellCommand should be it's own file if we're using SHELL for anything other than printing).
You don't have to go to the web site to approve a PR --
gh pr review NNN -a would do it
USAGE
gh pr review [<number> | <url> | <branch>] [flags]
FLAGS
-a, --approve Approve pull request
-b, --body string Specify the body of a review
-F, --body-file file Read body text from file (use "-" to read from standard input)
-c, --comment Comment on a pull request
-r, --request-changes Request changes on a pull request
INHERITED FLAGS
--help Show help for command
-R, --repo [HOST/]OWNER/REPO Select another repository using the [HOST/]OWNER/REPO format
EXAMPLES
# approve the pull request of the current branch
$ gh pr review --approve
# leave a review comment for the current branch
$ gh pr review --comment -b "interesting"
# add a review for a specific pull request
$ gh pr review 123
# request changes on a specific pull request
$ gh pr review 123 -r -b "needs more ASCII art"
LEARN MORE
Use 'gh <command> <subcommand> --help' for more information about a command.
Read the manual at https://cli.github.com/manual
I didn’t have gh, I got it from macports…but it hangs when I type by account “rmkaplan” in the gh auth login. So I can’t try this.
But if this lets me see the same opaque diffs that I can see on the website, it doesn’t address the problem that I am trying to solve. I’m trying to create a Medley-meaningful way of comparing the changes, and doing it without screwing around with my local git clone or working-Medley local file systems. I’m trying to avoid getting burned by git.
On Nov 21, 2021, at 10:39 AM, Larry Masinter @.***> wrote:
back to the workflow that started this issue (the ShellCommand character handling should be a separate issue; ShellCommand should be it's own file if we're using SHELL for anything other than printing). You don't have to go to the web site to approve a PR -- gh pr review NNN -a would do it
USAGE gh pr review [
| | ] [flags] FLAGS -a, --approve Approve pull request -b, --body string Specify the body of a review -F, --body-file file Read body text from file (use "-" to read from standard input) -c, --comment Comment on a pull request -r, --request-changes Request changes on a pull request
INHERITED FLAGS --help Show help for command -R, --repo [HOST/]OWNER/REPO Select another repository using the [HOST/]OWNER/REPO format
EXAMPLES
approve the pull request of the current branch
$ gh pr review --approve
leave a review comment for the current branch
$ gh pr review --comment -b "interesting"
add a review for a specific pull request
$ gh pr review 123
request changes on a specific pull request
$ gh pr review 123 -r -b "needs more ASCII art"
LEARN MORE Use 'gh
--help' for more information about a command. Read the manual at https://cli.github.com/manual — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974870725, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJPFKTTESKTGPGXUCKLUNE4FLANCNFSM5IM4ORTQ.
I was suggesting instead of "If the merge looks good, go back to github to click "approve"." use the gh command if you have it installed and authenticated.
Ahh, I see, that’s good.
But I have to figure out how to authenticate
On Nov 21, 2021, at 12:16 PM, Larry Masinter @.***> wrote:
I was suggesting instead of "If the merge looks good, go back to github to click "approve"." use the gh command if you have it installed and authenticated.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-974888344, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJLHHKASVW3UG3CMYR3UNFHQXANCNFSM5IM4ORTQ.
gh auth login --help
Authenticate with a GitHub host.
The default authentication mode is a web-based browser flow.
Alternatively, pass in a token on standard input by using `--with-token`.
The minimum required scopes for the token are: "repo", "read:org".
The --scopes flag accepts a comma separated list of scopes you want your gh credentials to have. If
absent, this command ensures that gh has access to a minimum set of scopes.
USAGE
gh auth login [flags]
FLAGS
-h, --hostname string The hostname of the GitHub instance to authenticate with
-s, --scopes strings Additional authentication scopes for gh to have
-w, --web Open a browser to authenticate
--with-token Read token from standard input
INHERITED FLAGS
--help Show help for command
EXAMPLES
# start interactive setup
$ gh auth login
# authenticate against github.com by reading the token from a file
$ gh auth login --with-token < mytoken.txt
# authenticate with a specific GitHub Enterprise Server instance
$ gh auth login --hostname enterprise.internal
LEARN MORE
Use 'gh <command> <subcommand> --help' for more information about a command.
Read the manual at https://cli.github.com/manual
28 days -- i think we need some timeouts on either request change or approve Pull Request 545 has no changes to Lisp code, it's documentation.
Reply to email about GITFNS for PR #684
there are a few files (not many) that got moved AND also edited.(but in different commits). This shows up in the git branch structure but might be lost when the changes are squashed. I moved internal/library to internal and changed all the references (using grep -lri internal/library) including FIX-MEDLEY-DIRS variable on internal/MEDLEY-UTILS). .
There are a couple of ways of dealing with renames and moves.
- With a few exceptions (README.md, CLtL2, font files) the same name has only one location (case insensitive). So that's a big clue.
- it would be possible to SHA1 all the files -- find moves, copies, and even possibly earlier dates.
On Fri, Feb 18, 2022 at 7:56 PM Nick Briggs [email protected] wrote: Looking for a way for Ron's tool to figure that out automatically based on what git will tell us. I think the --name-status should have sufficient info.
On Feb 18, 2022, at 7:51 PM, Larry Masinter <[email protected]> wrote:
I moved all the wd.Z files out into obsolete and did git mv fonts/xeroxprivate fonts/xerox
On Fri, Feb 18, 2022 at 6:41 PM Nick Briggs <[email protected]> wrote: A little googling suggest that maybe
git diff --name-status -C --find-copies-harder
might be more useful than the git diff --name-only... for finding what happened to files. "git help diff" will point you to what the status letters mean.
I'm not sure about the "infilep" equivalent.
-- Nick
On Feb 18, 2022, at 4:48 PM, Ron Kaplan <[email protected]> wrote:
Hi Nick,
Larry has been moving files around, I presume using a git mv command.
When I run my branch-to-branch comparision in Medley, I see all the files showing up as new files in the new directory, but I don’t see them as having been essentially deleted from the old directory. Logically, I think that’s what I should see, even though it would be nice to show directly that they have just been renamed—a separate issue.
Based on your previous suggestions, the way I’m determining the files to compare is to run
git merge-base branch1 branch2 (branch2 is origin/master for a pr compare)
followed by
git diff —name-only <merge> branch1
to get handles on all the different files
After that, for each file, I run
git show <merge>:file
git show branch1:file
and then capturing the bytes for comparison.
If I run this on Larry’s Xerox font branch (lmm16) against origin/master, I see all the obsolete/ entries but none of the originals.
Is there something else I should be doing to see where the file disappeared?
—Ron
[Separate question: I also have essentially a git infilep function (does a file exist in a branch?). I’m doing that by using the same git show command, and looking to see if what shows up is a “fatal” message, not a file. Is there a more direct way of testing whether a file exists on a branch?]
Running the command git diff --name-status -C --find-copies-harder branch1 for Larry's promote internal, one of its return lines is R067 internal/library/MEDLEY-UTILS internal/MEDLEY-UTILS suggesting that the file and its rename are only 67% the same?
What kind of a rename is that? Why does it call it a rename instead of a change?
I think it's only a change if the file stayed in the same place. Here he moved it from internal/library to internal -- which is probably a mistake... I don't think there should be any files at that level.
But if it’s a rename I would expect to see R100, if a file is just moved presumably its contents remain the same.
Otherwise, if the file was renamed or copied from one place to another and then edited, I would expect to see a delete and an add.
A file that was changed in place should show up as just a change (M for modified).
On Feb 22, 2022, at 11:50 PM, Nick Briggs @.***> wrote:
I think it's only a change if the file stayed in the same place. Here he moved it from internal/library to internal -- which is probably a mistake... I don't think there should be any files at that level.
— Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-1048520508, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJMALD7ET7UEGMIZA2LU4SGS7ANCNFSM5IM4ORTQ. You are receiving this because you were mentioned.
So I’m seeing the history of his actions?
It appears that when I see an R100, that the file only exists in its new location (although I should check that again), which is OK because there is no need to check the contents.
But if R067, then I assume that the proper thing to do is to get both files and compare them. But if it was renamed, can I actually retrieve the earlier unmoved/unchanged version?
On Feb 22, 2022, at 11:51 PM, Nick Briggs @.***> wrote:
and he changed it as well as moving it, so it wasn't an R100 ...
— Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-1048521290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJKNUSAYONFBXCUYY63U4SGX5ANCNFSM5IM4ORTQ. You are receiving this because you were mentioned.
Well you're seeing what git perceives the net effect of his actions was... though you can't tell if it was edit/rename or rename/edit (or any number of edits and renames) In both cases (R100 and R067) it will only exist in the new branch by the new name, but you should be able to use the same method of retrieving them as you do when you're comparing a modification (though the path names will be different in the old and new case...)
I don’t think the history is relevant to a pr compare, what matters is the current state of the new branch vs the state of the master. So I think it should be shown as an added file in one place, with whatever name it happens to have, and a delete in another. If something is R100, I show it in the table with its prior name and its new name, but there is only one file. You can see it, but you can’t do a content-compare.
On Feb 23, 2022, at 12:04 AM, Nick Briggs @.***> wrote:
Well you're seeing what git perceives the net effect of his actions was... though you can't tell if it was edit/rename or rename/edit (or any number of edits and renames) In both cases (R100 and R067) it will only exist in the new branch by the new name, but you should be able to use the same method of retrieving them as you do when you're comparing a modification (though the path names will be different in the old and new case...)
— Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-1048528930, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJPOEYRPAVQA4LPCHATU4SIKPANCNFSM5IM4ORTQ. You are receiving this because you were mentioned.
But for the promote-internal branch, when I allow for the R067, I run into another problem. It returns a list of 3465 lines (R’s, M’s, A’s), but the last 2 lines are:
"warning: only found copies from modified paths due to too many files.” "warning: you may want to set your diff.renameLimit variable to at least 10282 and retry the command.”
Looking at the pr on the web site, it says that there are 3463 changed files (and it chokes when you go down to review).
So maybe even the web site is not able to deal with this many changed files, and it’s summary is also not complete.
I can fix the medley code to watch for those lines and at least do the comparison for those (a table of 3465 entries most of which I assume are renames).
But maybe this kind of mass relocation is generally out of scope.
On Feb 23, 2022, at 12:10 AM, Ron Kaplan @.***> wrote:
I don’t think the history is relevant to a pr compare, what matters is the current state of the new branch vs the state of the master. So I think it should be shown as an added file in one place, with whatever name it happens to have, and a delete in another. If something is R100, I show it in the table with its prior name and its new name, but there is only one file. You can see it, but you can’t do a content-compare.
On Feb 23, 2022, at 12:04 AM, Nick Briggs @.*** @.***>> wrote:
Well you're seeing what git perceives the net effect of his actions was... though you can't tell if it was edit/rename or rename/edit (or any number of edits and renames) In both cases (R100 and R067) it will only exist in the new branch by the new name, but you should be able to use the same method of retrieving them as you do when you're comparing a modification (though the path names will be different in the old and new case...)
— Reply to this email directly, view it on GitHub https://github.com/Interlisp/medley/issues/575#issuecomment-1048528930, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSTUJPOEYRPAVQA4LPCHATU4SIKPANCNFSM5IM4ORTQ. You are receiving this because you were mentioned.
i could split this up into a series of PRs. In the case of MEDLEY-UTILS there were two changes (in separate commits).
The problem with gitfns only dealing in branches is that branches aren't stable -- they're names for commits where the name can be advanced by pushing (as happened a couple of times).