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

Problem when tree doesn't change but commit structure has changed #123

Open
ezyang opened this issue Nov 22, 2022 · 2 comments
Open

Problem when tree doesn't change but commit structure has changed #123

ezyang opened this issue Nov 22, 2022 · 2 comments

Comments

@ezyang
Copy link
Owner

ezyang commented Nov 22, 2022

Suppose you have commit A and B. You squash them into AB and run ghstack. This won't work; ghstack will say "well, B has the same tree as AB, so there's nothing to do." Even worse, when you're done, it will re check out B, undoing your AB merge.

@dan-zheng
Copy link

I think I've run into this issue.


I had a chain of commits, A-B-C-D.

I used git rebase -i B~ and squashed BC together: A-BC-D.

After rebasing, ghstack doesn't update the pull request for BC anymore.
https://github.com/google-research/triangulate/pull/16

Is there any way I can fix this?


Full git log below:

commit 7005442dd4f0c2be28d7e24e5ca14e1c91ff943f
Author: Dan Zheng <danielzheng@google.com>
Date:   Mon Aug 7 11:44:41 2023 -0700

    Update quoter.py test case.
    
    Add flags to quoter.py for conditional flag-dependent exception raising.
    
    Consider replacing quoter.py altogether in the future.
    
    ghstack-source-id: 21b43d1e4151c50ebf86c3cc84ddf458bcf17c8c
    Pull Request resolved: https://github.com/google-research/triangulate/pull/17

commit 24409a0b44b8ac97c75a4430a8b39c4c231d9545
Author: Dan Zheng <danielzheng@google.com>
Date:   Mon Aug 7 11:44:40 2023 -0700

    Update core logic and main binary.
    
    Use newly added utilities to improve core logic:
    - `ast_utils`: Improved AST matching for extracting illegal state expressions.
    - `instrumentation_utils`: Robust probe call insertion, instead of relying on
      `eval` and string interpolation.
    - `logging`: Prettier and useful logging statements.
    
    ---
    
    Previously, `main` used explicit flags like `bug_trap` (bug line number) to
    describe the subject program failure location. This led to long CLI invocations,
    which are error-prone and difficult to use.
    
    Now, `main` is designed like `pdb`, wrapping a standard Python script invocation:
    
    ```
    triangulate [flags...] subject -- [subject_args...]
    ```
    
    `main` runs the subject program with arguments. If an exception is thrown, the
    exception location is used as the bug line number. This `pdb`-like invocation mode
    is easier to use and test.
    
    `main` now does prettier logging as well, for status messages.
    
    ghstack-source-id: 4f16c3e131f0d3ab319f48ab5d06e78cab4fd29b
    Pull Request resolved: https://github.com/google-research/triangulate/pull/16

commit c0627945427756e929f0428eabbee302263eec18
Author: Dan Zheng <danielzheng@google.com>
Date:   Mon Aug 7 11:22:56 2023 -0700

    Add AST matching mini-library.
    
    Add an `AST` wrapper class with a method `select_nodes(selector)` method for
    returning all descendent AST nodes that match some predicate.
    
    Add AST node predicates:
    - `NodePredicate`: base class.
    - `All`: intersection combinator of multiple `NodePredicate`s.
    - `HasType(type)`: matches nodes with a given type.
    - `HasLocationInfo`: matches nodes that have line number information.
    - `OverlapsWithLineNumber(type)`: matches nodes that overlap with a line number.
    - `IsProbeStatement(type)`: matches nodes that are probe statements.
      - Currently matches all `triangulate_probe` function calls, which are
        generated via `instrumentation_utils.make_probe_call`.
    
    These will replace previous AST utilities for extracting `Assert` statements,
    which are less robust.
    
    ghstack-source-id: 4d23be5825365c89cd7cf5a58c6ea5eeb80c88dd
    Pull Request resolved: https://github.com/google-research/triangulate/pull/14

commit a85353e117c5c441bb7e8ed6e70bb4db632f41b2
Author: Dan Zheng <danielzheng@google.com>
Date:   Mon Aug 7 11:17:59 2023 -0700

    Add instrumentation utilities.
    
    Add a `probe` function for printing local variables (if defined).
    
    Add a `run_with_instrumentation` function for executing code and returning
    stdout and stderr as output.
    
    ghstack-source-id: ee48b5c8c5b9cb22b7177a638a9904e7963baa76
    Pull Request resolved: https://github.com/google-research/triangulate/pull/13

commit fbeee61d036e461da5664e893c31b5a7951f549a
Author: Dan Zheng <danielzheng@google.com>
Date:   Mon Aug 7 11:17:55 2023 -0700

    Add logging utilities.
    
    Add utilites for colored printing: convenience wrappers around termcolor.
    
    ghstack-source-id: 327a130c534ab13e452a4257a66e883003f5eac4
    Pull Request resolved: https://github.com/google-research/triangulate/pull/12

@dan-zheng
Copy link

Okay, I found a workaround.

I had a chain of commits, A-B-C-D.

I used git rebase -i B~ and squashed BC together: A-BC-D.

After rebasing, ghstack doesn't update the pull request for BC anymore. https://github.com/google-research/triangulate/pull/16

The workaround:

  • Rebase and amend the parent of the squashed commit: git rebase -i <squashed commit>~2.
    • I guess this updates "the tree".
  • Run ghstack again. Now PRs are updated, starting with the parent of the squashed commit.

I don't know how to apply this workaround if the squashed commit is the first PR in the chain.

ezyang added a commit that referenced this issue Dec 6, 2023
Fixes #123
Fixes #101

Signed-off-by: Edward Z. Yang <ezyang@meta.com>

[ghstack-poisoned]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants