ghstack should submit the current rev and lower in the stack and not the entire stack.

[nemith]

My workflow for sl (from years of hg inside FB) is to.

1. Code a lot of things that are somewhat interdependent. I tend to jump around a bit.
2. sl split to split those out into logical testable/reviewable stages.
3. Clean up each commit to make sure it's ready for review
4. Submit them as I clean them up.
5. Start next feature on top of the last one

So usually I am never just submitting one giant stack, but iterating on a current stack. I usually will submit one PR/Diff at a time and then work on fixing the next one and submit it.

Running sl ghstack submit however just took everything in my stack and made a PR for each one. This is not ideal as usually at the top of my stack i have a "scratch" commit of random stuff that needs to be split or cleaned up before submitting.

I think sl ghstack submit should either take a rev as an option or use the current rev in the stack I am in and submit all the revs below it and none of the ones on top.

$ cd testsap
$ ls
LICENSE
$ touch foo
$ sl add foo
$ sl commit -m "foo"
$ touch bar
$ sl add bar
$ sl commit -m "bar"
$ touch WIP
$ sl add WIP
$ sl commit -m "WIP"
$ sl
@  771ffce76  1 second ago  brandonbennett
│  WIP
│
o  50fe352f6  17 seconds ago  brandonbennett
│  bar
│
o  b2650f327  26 seconds ago  brandonbennett
│  foo
│
o  b3cd6c01e  63 seconds ago  remote/main

$ sl prev
0 files updated, 0 files merged, 1 files removed, 0 files unresolved
[50fe35] bar
$ sl ghstack submit
To https://github.com/nemith/testsap.git
 * [new branch]      81ee4b0a8e2bf26fa9ac116d62253718682479ca -> gh/nemith/0/head
 * [new branch]      b3cd6c01e96f7b2a50294d97cb8a60f327cd9875 -> gh/nemith/0/base
To https://github.com/nemith/testsap.git
 * [new branch]      58bc59f473a4b99988b70b9baf7b0876f374c24a -> gh/nemith/1/head
 * [new branch]      81ee4b0a8e2bf26fa9ac116d62253718682479ca -> gh/nemith/1/base
To https://github.com/nemith/testsap.git
 * [new branch]      6e3eaeef20b1e649a98503c8d625ceeda4f20a8d -> gh/nemith/2/head
 * [new branch]      58bc59f473a4b99988b70b9baf7b0876f374c24a -> gh/nemith/2/base
To https://github.com/nemith/testsap.git
 * [new branch]      d7acf5fb8de88c38e3048099678918f02d09ace0 -> gh/nemith/0/orig
 * [new branch]      7b1de853bc038f529ddbd20e3af03cbd16698af6 -> gh/nemith/1/orig
 * [new branch]      a4d69002cd52bde2e2678dd4bc13b30f731546e9 -> gh/nemith/2/orig

# Summary of changes (ghstack 0.6.0.sapling)

 - Created https://github.com/nemith/testsap/pull/3
 - Created https://github.com/nemith/testsap/pull/2
 - Created https://github.com/nemith/testsap/pull/1

Facebook employees can import your changes by running
(on a Facebook machine):

    ghimport -s https://github.com/nemith/testsap/pull/3

If you want to work on this diff stack on another machine:

    ghstack checkout https://github.com/nemith/testsap/pull/3

You can see that a PR was created for WIP which is wrong and will almost (in my workflow) never be used. Right now i need to close that WIP PR as it will never ever be reviewable. (also showing the world the mess that is my head and development processes).

Looking through the options of hgstack this doesn't seem to be an option either.

[nemith]

sl pr seems to do the right thing:

(on a new stack created just like before)

$sl pr create
pushing 2 to https://github.com/nemith/testsap.git
created new pull request: https://github.com/nemith/testsap/pull/4
created new pull request: https://github.com/nemith/testsap/pull/5
updated body for https://github.com/nemith/testsap/pull/4
updated body for https://github.com/nemith/testsap/pull/5
$ sl
o  3277c0bfd  90 seconds ago  brandonbennett
│  WIP
│
@  555e61920  95 seconds ago  brandonbennett  #5
│  bar
│
o  c24eccbb9  100 seconds ago  brandonbennett  #4
│  foo
│
│ o  29dda6700  3 minutes ago  brandonbennett  #3
│ │  WIP
│ │
│ o  bb44826ed  3 minutes ago  brandonbennett  #2
│ │  bar
│ │
│ o  aabaae8cb  3 minutes ago  brandonbennett  #1
├─╯  foo
│
o  b3cd6c01e  11 minutes ago  remote/main

[nemith]

Seems like this has been brought up on ghstack already: ezyang/ghstack#101

[nemith]

Looks like HEAD in ghstacks is a bit different than what i literally translates to when doing branless/trunk development.

Here is a log of sl ghstack submit --verbose
https://gist.github.com/nemith/a141213e602c989c7ce1f4ec91807023

The interesting line is $ /opt/homebrew/bin/sl log -r 'max(descendants(.))' -T '{node}' in which sapling_shell.py is running to get a subsitute for for HEAD:

$ sl log -T '{node}' -r 'ancestor(., main)'
b3cd6c01e96f7b2a50294d97cb8a60f327cd9875
$ sl log -r 'max(descendants(.))' -T '{node}'
4e704e7f0fa5a4fc0a7f0a67c3ad284d378349c0
git --git-dir /Users/brandonbennett/Code/testsap/.sl/store/git rev-list --header '^b3cd6c01e96f7b2a50294d97cb8a60f327cd9875' faa7bc9df57fe920a29b41902e7a940c55224c8e
faa7bc9df57fe920a29b41902e7a940c55224c8e
tree e1f50ed2aa69f8bd4dc587c3c65b70a83429dddc
parent 6df3d4a4f31b5b9e21702f1b434c03bd8d1ca9de
author Brandon Bennett <[email protected]> 1669749869 -0700
committer Brandon Bennett <[email protected]> 1669749869 -0700

    spam
6df3d4a4f31b5b9e21702f1b434c03bd8d1ca9de
tree 63789fac9a35f48332930d8af207b9e5e34b0ff6
parent 1cabefc0773164eb309ee84567f3492d3f936c2f
author Brandon Bennett <[email protected]> 1669749811 -0700
committer Brandon Bennett <[email protected]> 1669749811 -0700

    quux
1cabefc0773164eb309ee84567f3492d3f936c2f
tree defbe41b8836c18ffeaa72c5c748cdd1e0095911
parent 3e3f4c1dbbd23b9a70f815441bbc9d6688844d4b
author Brandon Bennett <[email protected]> 1669749796 -0700
committer Brandon Bennett <[email protected]> 1669749796 -0700

    bar
3e3f4c1dbbd23b9a70f815441bbc9d6688844d4b
tree dae37a0b9fecafee09c60547d255659eb9b5ed44
parent b3cd6c01e96f7b2a50294d97cb8a60f327cd9875
author Brandon Bennett <[email protected]> 1669749783 -0700
committer Brandon Bennett <[email protected]> 1669749783 -0700

    foo
$  sl
o  4e704e7f0  8 minutes ago  brandonbennett  #9
│  spam
│
o  8c2719dfe  8 minutes ago  brandonbennett  #8
│  quux
│
@  e919dbdd4  9 minutes ago  brandonbennett  #7
│  bar
│
o  e76195eab  9 minutes ago  brandonbennett  #6
│  foo
│
│ o  d09be4575  12 minutes ago  brandonbennett  #3
│ │  WIP
│ │
│ o  1274c119e  12 minutes ago  brandonbennett  #2
│ │  bar
│ │
│ o  da3f642f8  12 minutes ago  brandonbennett  #1
├─╯  foo
│
│ o  3277c0bfd  Today at 09:40  brandonbennett
│ │  WIP
│ │
│ o  555e61920  Today at 09:40  brandonbennett  #5
│ │  bar
│ │
│ o  c24eccbb9  Today at 09:40  brandonbennett  #4
├─╯  foo
│
o  b3cd6c01e  Today at 09:30  remote/main

https://github.com/facebook/sapling/blob/main/eden/scm/ghstack/sapling_shell.py#L67-L74

I think this is not the correct translation for HEAD in call cases and HEAD in this case should actually mean "current commit" (i.e .) and not max(descendants(.)). I am not sure that logic holds up for the rest of GHstack git calls so I am not sure it's a simple change as change max(descendants(.)) to (.)`.

Thoughts?