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.

Sign up for GitHub

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

Jump to bottom

Broken pipe when running jj rebase 2>&1 | head causes stale working copy #4239

Closed
ilyagr opened this issue Aug 8, 2024 · 6 comments · Fixed by #4517
Closed

Broken pipe when running jj rebase 2>&1 | head causes stale working copy #4239

ilyagr opened this issue Aug 8, 2024 · 6 comments · Fixed by #4517
Assignees
@essiene
Labels
🐛bug Something isn't working

Comments

@ilyagr
Copy link
Collaborator

ilyagr commented Aug 8, 2024 •
edited
Loading

I tried the following command in the situation where the rebase would cause the conflict.

(The goal was to use the command in a demo but shorten the output)

The result was, surprisingly, a stale working copy for the following operation. This is not a concurrency bug, it is completely repeatable for me.

$ jj rebase -s third -d first 2>&1 |  head -n 3 # Shorten the long message
Rebased 2 commits
New conflicts appeared in these commits:
  lnyvokkk 34755ec1 third | (conflict) third
$ jj rebase -s second -d third
Error: The working copy is stale (not updated since operation 6c9667ecda1f).
Hint: Run `jj workspace update-stale` to update it.
See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-working-copy for more information.

I'm guessing that the first jj rebase aborted in the middle of an operation because of a broken pipe, and this caused a corrupted working copy. I'm not 100% sure this is a bug, but it's certainly was surprising to me.
ilyagr added a commit to ilyagr/jj that referenced this issue Aug 8, 2024
@ilyagr
demos: redact one of the conflict outputs
48eadb8 
I would prefer to shorten it, but I ran into
jj-vcs#4239
ilyagr added a commit to ilyagr/jj that referenced this issue Aug 8, 2024
@ilyagr
demos: redact one of the conflict outputs
3d16f20 
I would prefer to shorten it, but I ran into
jj-vcs#4239
@yuja
Copy link
Collaborator

yuja commented Aug 8, 2024

I think this is expected after failed working-copy updates.

That said, this particular scenario can be mitigated by reordering update_working_copy() and report_repo_changes().

https://github.com/martinvonz/jj/blob/b3ede64472f9cd9290cb6a3f8776caac69dca793/cli/src/cli_util.rs#L1424-L1430

@PhilipMetzger PhilipMetzger added the 🐛bug Something isn't working label Aug 8, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Aug 9, 2024
@ilyagr
demos: redact one of the conflict outputs
a5a5b26 
I would prefer to shorten it, but I ran into
jj-vcs#4239
@ilyagr
Copy link
Collaborator Author

ilyagr commented Aug 10, 2024

I'm not sure what's best here, but here are two unbaked thoughts.

I wonder if, in the ideal world, we could make it so that as many errors as possible are either handled before the mutation starts or after it is over. That sounds like a lot of busywork, though.

I suppose we could treat BrokenPipe-s specially. It's a bit annoying that we can't judge the error by their larger type; some IOErrors are critical (writing to the store failed), and some are just BrokenPipe-s.

@yuja
Copy link
Collaborator

yuja commented Aug 10, 2024

I think many of the interrupted mutation problems are addressed by transaction in jj. Working-copy updates and git exports are two major sources of inconsistency, and I think we can avoid broken pipes in that region relatively easily.

@essiene essiene self-assigned this Aug 25, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Sep 5, 2024
@ilyagr
demos: redact one of the conflict outputs
caeb9aa 
I would prefer to shorten it, but I ran into
jj-vcs#4239
ilyagr added a commit to ilyagr/jj that referenced this issue Sep 5, 2024
@ilyagr
demos: redact one of the conflict outputs
a2d45ea 
I would prefer to shorten it instead of fully redacting, but I ran into
jj-vcs#4239
ilyagr added a commit to ilyagr/jj that referenced this issue Sep 5, 2024
@ilyagr
demos: redact one of the conflict outputs
a2563d5 
I would prefer to shorten it instead of fully redacting, but I ran into
jj-vcs#4239
ilyagr added a commit that referenced this issue Sep 6, 2024
@ilyagr
demos: redact one of the conflict outputs
d3b0f1b 
I would prefer to shorten it instead of fully redacting, but I ran into
#4239
essiene added a commit that referenced this issue Sep 21, 2024
@essiene
cli: reorder updating and reporting for consistency.
b4c6146 
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
@essiene essiene mentioned this issue Sep 21, 2024
Merged
4 tasks
@essiene
Copy link
Collaborator

essiene commented Sep 21, 2024

So I put together a draft in #4517, that just moves the call to self.report_repo_changes(...) after the may_update_working_copy(...) block.

It changes the ordering of messages on the CLI. I'm not sure if I think the ordering is better or worse, but it's very different. Is this fine? Should we look for another way to solve this?

thoughts?

@yuja
Copy link
Collaborator

yuja commented Sep 21, 2024

changes the ordering of messages on the CLI. I'm not sure if I think the ordering is better or worse,

I think it's okay, and I personally like the new order since conflicts would be the thing the user will have to address next.

@essiene
Copy link
Collaborator

essiene commented Sep 22, 2024

changes the ordering of messages on the CLI. I'm not sure if I think the ordering is better or worse,

I think it's okay, and I personally like the new order since conflicts would be the thing the user will have to address next.

Okie dokie. I'll mark the PR as ready then. Thanks.

essiene added a commit that referenced this issue Sep 22, 2024
@essiene
cli: reorder updating and reporting for consistency.
9979ea4 
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
essiene added a commit that referenced this issue Sep 22, 2024
@essiene
cli: reorder updating and reporting for consistency.
cb909ba 
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
essiene added a commit that referenced this issue Sep 22, 2024
@essiene
cli: reorder updating and reporting for consistency.
1795fed 
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
essiene added a commit that referenced this issue Sep 22, 2024
@essiene
cli: reorder updating and reporting for consistency.
77772ce 
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
@BrewTestBot BrewTestBot mentioned this issue Oct 2, 2024
Merged
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Oct 8, 2024
@0-wiz-0
jj: update to 0.22.0.
68554f2 
## [0.22.0] - 2024-10-02

### Breaking changes

* Fixing [#4239](jj-vcs/jj#4239) means the
  ordering of some messages have changed.

* Invalid `ui.graph.style` configuration is now an error.

* The builtin template `branch_list` has been renamed to `bookmark_list` as part
  of the `jj branch` deprecation.

### Deprecations

* `jj branch` has been deprecated in favor of `jj bookmark`.

  **Rationale:** Jujutsu's branches don't behave like Git branches, which a
  confused many newcomers, as they expected a similar behavior given the name.
  We've renamed them to "bookmarks" to match the actual behavior, as we think
  that describes them better, and they also behave similar to Mercurial's
  bookmarks.

* `jj obslog` is now called `jj evolution-log`/`jj evolog`. `jj obslog` remains
  as an alias.

* `jj unsquash` has been deprecated in favor of `jj squash` and
  `jj diffedit --restore-descendants`.

  **Rationale:** `jj squash` can be used in interactive mode to pull
  changes from one commit to another, including from a parent commit
  to a child commit. For fine-grained dependent diffs, such as when
  the parent and the child commits must successively modify the same
  location in a file, `jj diffedit --restore-descendants` can be used
  to set the parent commit to the desired content without altering the
  content of the child commit.

* The `git.push-branch-prefix` config has been deprecated in favor of
  `git.push-bookmark-prefix`.

* `conflict()` and `file()` revsets have been renamed to `conflicts()` and `files()`
  respectively. The old names are still around and will be removed in a future
  release.

### New features

* The new config option `snapshot.auto-track` lets you automatically track only
  the specified paths (all paths by default). Use the new `jj file track`
  command to manually tracks path that were not automatically tracked. There is
  no way to list untracked files yet. Use `git status` in a colocated workspace
  as a workaround.
  [#323](jj-vcs/jj#323)

* `jj fix` now allows fixing unchanged files with the `--include-unchanged-files` flag. This
  can be used to more easily introduce automatic formatting changes in a new
  commit separate from other changes.

* `jj workspace add` now accepts a `--sparse-patterns=<MODE>` option, which
  allows control of the sparse patterns for a newly created workspace: `copy`
  (inherit from parent; default), `full` (full working copy), or `empty` (the
  empty working copy).

* New command `jj workspace rename` that can rename the current workspace.

* `jj op log` gained an option to include operation diffs.

* `jj git clone` now accepts a `--remote <REMOTE NAME>` option, which
  allows to set a name for the remote instead of using the default
  `origin`.

* `jj op undo` now reports information on the operation that has been undone.

* `jj squash`: the `-k` flag can be used as a shorthand for `--keep-emptied`.

* CommitId / ChangeId template types now support `.normal_hex()`.

* `jj commit` and `jj describe` now accept `--author` option allowing to quickly change
  author of given commit.

* `jj diffedit`, `jj abandon`, and `jj restore` now accept a `--restore-descendants`
  flag. When used, descendants of the edited or deleted commits will keep their original
  content.

* `jj git fetch -b <remote-git-branch-name>` will now warn if the branch(es)
   can not be found in any of the specified/configured remotes.

* `jj split` now lets the user select all changes in interactive mode. This may be used
  to keeping all changes into the first commit while keeping the current commit
  description for the second commit (the newly created empty one).

* Author and committer names are now yellow by default.

### Fixed bugs

* Update working copy before reporting changes. This prevents errors during reporting
  from leaving the working copy in a stale state.

* Fixed panic when parsing invalid conflict markers of a particular form.
  ([#2611](jj-vcs/jj#2611))

* Editing a hidden commit now makes it visible.

* The `present()` revset now suppresses missing working copy error. For example,
  `present(@)` evaluates to `none()` if the current workspace has no
  working-copy commit.
tmeijn pushed a commit to tmeijn/dotfiles that referenced this issue Oct 18, 2024
@tmeijn
build(deps): update martinvonz/jj to v0.22.0
eb82ae5 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [martinvonz/jj](https://github.com/martinvonz/jj) | minor | `v0.21.0` -> `v0.22.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>martinvonz/jj (martinvonz/jj)</summary>

### [`v0.22.0`](https://github.com/martinvonz/jj/releases/tag/v0.22.0)

[Compare Source](jj-vcs/jj@v0.21.0...v0.22.0)

##### Breaking changes

-   Fixing [#&#8203;4239](jj-vcs/jj#4239) means the
    ordering of some messages have changed.

-   Invalid `ui.graph.style` configuration is now an error.

-   The builtin template `branch_list` has been renamed to `bookmark_list` as part
    of the `jj branch` deprecation.

##### Deprecations

-   `jj branch` has been deprecated in favor of `jj bookmark`.

    **Rationale:** Jujutsu's branches don't behave like Git branches, which a
    confused many newcomers, as they expected a similar behavior given the name.
    We've renamed them to "bookmarks" to match the actual behavior, as we think
    that describes them better, and they also behave similar to Mercurial's
    bookmarks.

-   `jj obslog` is now called `jj evolution-log`/`jj evolog`. `jj obslog` remains
    as an alias.

-   `jj unsquash` has been deprecated in favor of `jj squash` and
    `jj diffedit --restore-descendants`.

    **Rationale:** `jj squash` can be used in interactive mode to pull
    changes from one commit to another, including from a parent commit
    to a child commit. For fine-grained dependent diffs, such as when
    the parent and the child commits must successively modify the same
    location in a file, `jj diffedit --restore-descendants` can be used
    to set the parent commit to the desired content without altering the
    content of the child commit.

-   The `git.push-branch-prefix` config has been deprecated in favor of
    `git.push-bookmark-prefix`.

-   `conflict()` and `file()` revsets have been renamed to `conflicts()` and `files()`
    respectively. The old names are still around and will be removed in a future
    release.

##### New features

-   The new config option `snapshot.auto-track` lets you automatically track only
    the specified paths (all paths by default). Use the new `jj file track`
    command to manually tracks path that were not automatically tracked. There is
    no way to list untracked files yet. Use `git status` in a colocated workspace
    as a workaround.
    [#&#8203;323](jj-vcs/jj#323)

-   `jj fix` now allows fixing unchanged files with the `--include-unchanged-files` flag. This
    can be used to more easily introduce automatic formatting changes in a new
    commit separate from other changes.

-   `jj workspace add` now accepts a `--sparse-patterns=<MODE>` option, which
    allows control of the sparse patterns for a newly created workspace: `copy`
    (inherit from parent; default), `full` (full working copy), or `empty` (the
    empty working copy).

-   New command `jj workspace rename` that can rename the current workspace.

-   `jj op log` gained an option to include operation diffs.

-   `jj git clone` now accepts a `--remote <REMOTE NAME>` option, which
    allows to set a name for the remote instead of using the default
    `origin`.

-   `jj op undo` now reports information on the operation that has been undone.

-   `jj squash`: the `-k` flag can be used as a shorthand for `--keep-emptied`.

-   CommitId / ChangeId template types now support `.normal_hex()`.

-   `jj commit` and `jj describe` now accept `--author` option allowing to quickly change
    author of given commit.

-   `jj diffedit`, `jj abandon`, and `jj restore` now accept a `--restore-descendants`
    flag. When used, descendants of the edited or deleted commits will keep their original
    content.

-   `jj git fetch -b <remote-git-branch-name>` will now warn if the branch(es)
    can not be found in any of the specified/configured remotes.

-   `jj split` now lets the user select all changes in interactive mode. This may be used
    to keeping all changes into the first commit while keeping the current commit
    description for the second commit (the newly created empty one).

-   Author and committer names are now yellow by default.

##### Fixed bugs

-   Update working copy before reporting changes. This prevents errors during reporting
    from leaving the working copy in a stale state.

-   Fixed panic when parsing invalid conflict markers of a particular form.
    ([#&#8203;2611](jj-vcs/jj#2611))

-   Editing a hidden commit now makes it visible.

-   The `present()` revset now suppresses missing working copy error. For example,
    `present(@&#8203;)` evaluates to `none()` if the current workspace has no
    working-copy commit.

##### Contributors

Thanks to the people who made this release happen!

-   Austin Seipp ([@&#8203;thoughtpolice](https://github.com/thoughtpolice))
-   Danny Hooper ([@&#8203;hooper](https://github.com/hooper))
-   Emily Shaffer ([@&#8203;nasamuffin](https://github.com/nasamuffin))
-   Essien Ita Essien ([@&#8203;essiene](https://github.com/essiene))
-   Ethan Brierley ([@&#8203;eopb](https://github.com/eopb))
-   Ilya Grigoriev ([@&#8203;ilyagr](https://github.com/ilyagr))
-   Kevin Liao ([@&#8203;kevincliao](https://github.com/kevincliao))
-   Lukas Wirth ([@&#8203;Veykril](https://github.com/Veykril))
-   Martin von Zweigbergk ([@&#8203;martinvonz](https://github.com/martinvonz))
-   Mateusz Mikuła ([@&#8203;mati865](https://github.com/mati865))
-   mlcui ([@&#8203;mlcui-corp](https://github.com/mlcui-corp))
-   Philip Metzger ([@&#8203;PhilipMetzger](https://github.com/PhilipMetzger))
-   Samuel Tardieu ([@&#8203;samueltardieu](https://github.com/samueltardieu))
-   Stephen Jennings ([@&#8203;jennings](https://github.com/jennings))
-   Tyler Goffinet ([@&#8203;qubitz](https://github.com/qubitz))
-   Vamsi Avula ([@&#8203;avamsi](https://github.com/avamsi))
-   Yuya Nishihara ([@&#8203;yuja](https://github.com/yuja))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@essiene essiene

Labels
🐛bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: reorder updating and reporting for consistency. jj-vcs/jj
4 participants
@essiene @yuja @ilyagr @PhilipMetzger