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

git diff output incompatibility with BSD patch for new files #5049

Closed
botovq opened this issue Dec 7, 2024 · 0 comments
Closed

git diff output incompatibility with BSD patch for new files #5049

botovq opened this issue Dec 7, 2024 · 0 comments
Labels
🐛bug Something isn't working polish🪒🐃 Make existing features more convenient and more consistent

Comments

@botovq
Copy link

botovq commented Dec 7, 2024

Description

The diff hunk header between two pairs of @ for a newly created file indicates that it applies to the first line of /dev/null file and confuses the patch implementations on FreeBSD and OpenBSD which then ask for a filename to be specified when one tries to apply the patch.

Steps to Reproduce the Problem

Create and commit a file containing a single line, generate patches from that commit and try to apply the patches in an empty directory.

Git's output has a hunk indicator @@ -0,0 +1 @@:

$ git show
[...]
diff --git a/hello b/hello
new file mode 100644
index 0000000..ce01362
--- /dev/null
+++ b/hello
@@ -0,0 +1, @@
+hello

whereas the equivalent output of jj has @@ -1,0 +1,1 @@:

$ jj show --git -r @-
[...]
diff --git a/hello b/hello
new file mode 100644
index 0000000000..ce01362503
--- /dev/null
+++ b/hello
@@ -1,0 +1,1 @@
+hello

The problem is the first -1 here. The BSD's patch implementations (derived from Larry Wall's patch) special case @@ 0 and use that as a hint that a new file needs to be created. Whether that's a great approach is a different question, but it has been this way for more than thirty years. If it's not @@ 0, it doesn't find the file and enters interactive mode, which doesn't work.

A simlar problem exists for the reverted diff, but it doesn't confuse BSD patch.

I think the following passage in the POSIX 2024 specification indicates that git's behavior is the expected one:

"%1d,%1d", <beginning line number>, <number of lines>

otherwise. If a range is empty, its beginning line number shall be the number of the line just before the range, or 0 if the empty range starts the file.

Expected Behavior

The diffs generated by git and jujutsu apply cleanly for the system-provided patch and create files as needed.

Actual Behavior

Attempting to apply the diffs in an empty directory results in the following ouptut on FreeBSD (tested on 14.2) and OpenBSD (tested on 7.6):

$ patch < /tmp/jj.diff
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Commit ID: 58258525c7bc0bb5afa9c672df4570600da27803
|Change ID: nzylvuozztzlpxkovltwqukupmmzwmlw
|Bookmarks: master master@git
|Author: Theo Buehler <[email protected]> (2024-12-07 22:04:11)
|Committer: Theo Buehler <[email protected]> (2024-12-07 22:04:11)
|
|    msg
|
|diff --git a/hello b/hello
|new file mode 100644
|index 0000000000..ce01362503
|--- /dev/null
|+++ b/hello
--------------------------
File to patch:

and as you can see, you're asked to enter the file name. This won't work of course since the file doesn't exist yet:

File to patch: hello
No file found--skip this patch? [n] n
patch: **** can't find hello

The diff generated by git on the other hand applies cleanly:

$ patch < /tmp/git.diff
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|commit 58258525c7bc0bb5afa9c672df4570600da27803
|Author: Theo Buehler <[email protected]>
|Date:   Sat Dec 7 22:04:11 2024 +0100
|
|    msg
|
|diff --git a/hello b/hello
|new file mode 100644
|index 0000000..ce01362
|--- /dev/null
|+++ b/hello
--------------------------
(Creating file hello...)
Patching file hello using Plan A...
Empty context always matches.
Hunk #1 succeeded at 1.
done

Editing the patch and changing the -1 to 0 or using GNU's patch works around this issue.

Specifications

  • Platform: FreeBSD 14.x, OpenBSD 7.6
  • Version: Affects jj versions 0.22 to 0.24 at least, probably present for much longer.
@PhilipMetzger PhilipMetzger added the polish🪒🐃 Make existing features more convenient and more consistent label Dec 7, 2024
@yuja yuja added the 🐛bug Something isn't working label Dec 8, 2024
yuja added a commit to yuja/jj that referenced this issue Dec 8, 2024
…hunks

Both Mercurial and Git (xdiff) have a special case for empty hunks.

https://repo.mercurial-scm.org/hg/rev/2b1ec74c961f

I also changed the internal line numbers to start from 0 so we wouldn't have
to think about whether "N - 1" would underflow.

Fixes jj-vcs#5049
@yuja yuja closed this as completed in 8c4ba4a Dec 9, 2024
tmeijn pushed a commit to tmeijn/dotfiles that referenced this issue Jan 3, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [martinvonz/jj](https://github.com/martinvonz/jj) | minor | `v0.24.0` -> `v0.25.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.25.0`](https://github.com/jj-vcs/jj/releases/tag/v0.25.0)

[Compare Source](jj-vcs/jj@v0.24.0...v0.25.0)

##### About

jj is a Git-compatible version control system that is both simple and powerful. See the [installation instructions](https://jj-vcs.github.io/jj/v0.25.0/install-and-setup/) to get started.

##### Release highlights

It's the holidays, and this release was overall pretty quiet, without many major
changes. Two select improvements:

-   Improvements to configuration management, including support for [conditional
    variables](https://jj-vcs.github.io/jj/v0.25.0/config#conditional-variables) in config files.

-   Large files in the working copy will no longer cause commands to fail; instead
    the large files will remain intact but untracked in the working copy.

##### Breaking changes

-   Configuration variables are no longer "stringly" typed. For example, `true` is
    not converted to a string `"true"`, and vice versa.

-   The following configuration variables are now parsed strictly:
    `colors.<labels>`, `git.abandon-unreachable-commits`,
    `git.auto-local-bookmark`, `git.push-bookmark-prefix`, `revsets.log`,
    `revsets.short-prefixes` `signing.backend`, `operation.hostname`,
    `operation.username`, `ui.allow-init-native`, `ui.color`,
    `ui.default-description`, `ui.progress-indicator`, `ui.quiet`, `user.email`,
    `user.name`

-   `jj config list` now prints inline tables `{ key = value, .. }` literally.
    Inner items of inline tables are no longer merged across configuration files.
    See [the table syntax documentation](https://jj-vcs.github.io/jj/v0.25.0/config#dotted-style-headings-and-inline-tables) for details.

-   `jj config edit --user` now opens a file even if `$JJ_CONFIG` points to a
    directory. If there are multiple config files, the command will fail.

-   `jj config set` no longer accepts a bare string value that looks like a TOML
    expression. For example, `jj config set NAME '[foo]'` must be quoted as `jj
    config set NAME '"[foo]"'`.

-   The deprecated `[alias]` config section is no longer respected. Move command
    aliases to the `[aliases]` section.

-   `jj absorb` now abandons the source commit if it becomes empty and has no
    description.

##### Deprecations

-   `--config-toml=TOML` is deprecated in favor of `--config=NAME=VALUE` and
    `--config-file=PATH`.

-   The `Signature.username()` template method is deprecated for
    `Signature().email().local()`.

##### New features

-   `jj` command no longer fails due to new working-copy files larger than the
    `snapshot.max-new-file-size` config option. It will print a warning and large
    files will be left untracked.

-   Configuration files now support [conditional variables](https://jj-vcs.github.io/jj/v0.25.0/config/#conditional-variables).

-   New command options `--config=NAME=VALUE` and `--config-file=PATH` to set
    string value without quoting and to load additional configuration from files.

-   Templates now support the `>=`, `>`, `<=`, and `<` relational operators for
    `Integer` types.

-   A new Email template type is added. `Signature.email()` now returns an Email
    template type instead of a String.

-   Adds a new template alias `commit_timestamp(commit)` which defaults to the
    committer date.

-   Conflict markers are now allowed to be longer than 7 characters, allowing
    conflicts to be materialized and parsed correctly in files which already
    contain lines that look like conflict markers.

-   New `$marker_length` variable to allow merge tools to support longer conflict
    markers (equivalent to "%L" for Git merge drivers).

-   `jj describe` now accepts a `JJ: ignore-rest` line that ignores everything
    below it, similar to a "scissor line" in git. When editing multiple commits,
    only ignore until the next `JJ: describe` line.

##### Fixed bugs

-   The `$NO_COLOR` environment variable must now be non-empty to be respected.

-   Fixed incompatible rendering of empty hunks in git/unified diffs. [#&#8203;5049](jj-vcs/jj#5049)

-   Fixed performance of progress bar rendering when fetching from Git remote. [#&#8203;5057](jj-vcs/jj#5057)

-   `jj config path --user` no longer creates new file at the default config path.

-   On Windows, workspace paths (printed by `jj root`) no longer use UNC-style
    `\\?\` paths unless necessary.

-   On Windows, `jj git clone` now converts local Git remote path to
    slash-separated path.

-   `jj resolve` no longer removes the executable bit on resolved files when using
    an external merge tool.

##### Contributors

Thanks to the people who made this release happen!

-   Alex Stefanov ([@&#8203;umnikos](https://github.com/umnikos))
-   Anton Älgmyr ([@&#8203;algmyr](https://github.com/algmyr))
-   Austin Seipp ([@&#8203;thoughtpolice](https://github.com/thoughtpolice))
-   Benjamin Tan ([@&#8203;bnjmnt4n](https://github.com/bnjmnt4n))
-   Bryce Berger ([@&#8203;bryceberger](https://github.com/bryceberger))
-   Daniel Ploch ([@&#8203;torquestomp](https://github.com/torquestomp))
-   David Crespo ([@&#8203;david-crespo](https://github.com/david-crespo))
-   George Tsiamasiotis ([@&#8203;gtsiam](https://github.com/gtsiam))
-   Jochen Kupperschmidt ([@&#8203;homeworkprod](https://github.com/homeworkprod))
-   Keane Nguyen ([@&#8203;keanemind](https://github.com/keanemind))
-   Martin von Zweigbergk ([@&#8203;martinvonz](https://github.com/martinvonz))
-   Matt Kulukundis ([@&#8203;fowles](https://github.com/fowles))
-   Milo Moisson ([@&#8203;mrnossiom](https://github.com/mrnossiom))
-   petricavalry ([@&#8203;petricavalry](https://github.com/petricavalry))
-   Philip Metzger ([@&#8203;PhilipMetzger](https://github.com/PhilipMetzger))
-   Remo Senekowitsch ([@&#8203;senekor](https://github.com/senekor))
-   Scott Taylor ([@&#8203;scott2000](https://github.com/scott2000))
-   Shane Sveller ([@&#8203;shanesveller](https://github.com/shanesveller))
-   Stephen Jennings ([@&#8203;jennings](https://github.com/jennings))
-   Tim Janik ([@&#8203;tim-janik](https://github.com/tim-janik))
-   Vamsi Avula ([@&#8203;avamsi](https://github.com/avamsi))
-   Waleed Khan ([@&#8203;arxanas](https://github.com/arxanas))
-   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:eyJjcmVhdGVkSW5WZXIiOiIzOS44Ni40IiwidXBkYXRlZEluVmVyIjoiMzkuODYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jan 7, 2025
## [0.25.0] - 2025-01-01

### Release highlights

It's the holidays, and this release was overall pretty quiet, without many major
changes. Two select improvements:

* Improvements to configuration management, including support for [conditional
  variables](docs/config.md#conditional-variables) in config files.

* Large files in the working copy will no longer cause commands to fail; instead
  the large files will remain intact but untracked in the working copy.

### Breaking changes

* Configuration variables are no longer "stringly" typed. For example, `true` is
  not converted to a string `"true"`, and vice versa.

* The following configuration variables are now parsed strictly:
  `colors.<labels>`, `git.abandon-unreachable-commits`,
  `git.auto-local-bookmark`, `git.push-bookmark-prefix`, `revsets.log`,
  `revsets.short-prefixes` `signing.backend`, `operation.hostname`,
  `operation.username`, `ui.allow-init-native`, `ui.color`,
  `ui.default-description`, `ui.progress-indicator`, `ui.quiet`, `user.email`,
  `user.name`

* `jj config list` now prints inline tables `{ key = value, .. }` literally.
  Inner items of inline tables are no longer merged across configuration files.
  See [the table syntax
  documentation](docs/config.md#dotted-style-headings-and-inline-tables) for
  details.

* `jj config edit --user` now opens a file even if `$JJ_CONFIG` points to a
  directory. If there are multiple config files, the command will fail.

* `jj config set` no longer accepts a bare string value that looks like a TOML
  expression. For example, `jj config set NAME '[foo]'` must be quoted as `jj
  config set NAME '"[foo]"'`.

* The deprecated `[alias]` config section is no longer respected. Move command
  aliases to the `[aliases]` section.

* `jj absorb` now abandons the source commit if it becomes empty and has no
  description.

### Deprecations

* `--config-toml=TOML` is deprecated in favor of `--config=NAME=VALUE` and
  `--config-file=PATH`.

* The `Signature.username()` template method is deprecated for
  `Signature().email().local()`.

### New features

* `jj` command no longer fails due to new working-copy files larger than the
  `snapshot.max-new-file-size` config option. It will print a warning and large
  files will be left untracked.

* Configuration files now support [conditional
  variables](docs/config.md#conditional-variables).

* New command options `--config=NAME=VALUE` and `--config-file=PATH` to set
  string value without quoting and to load additional configuration from files.

* Templates now support the `>=`, `>`, `<=`, and `<` relational operators for
  `Integer` types.

* A new Email template type is added. `Signature.email()` now returns an Email
  template type instead of a String.

* Adds a new template alias `commit_timestamp(commit)` which defaults to the
  committer date.

* Conflict markers are now allowed to be longer than 7 characters, allowing
  conflicts to be materialized and parsed correctly in files which already
  contain lines that look like conflict markers.

* New `$marker_length` variable to allow merge tools to support longer conflict
  markers (equivalent to "%L" for Git merge drivers).

* `jj describe` now accepts a `JJ: ignore-rest` line that ignores everything
  below it, similar to a "scissor line" in git. When editing multiple commits,
  only ignore until the next `JJ: describe` line.

### Fixed bugs

* The `$NO_COLOR` environment variable must now be non-empty to be respected.

* Fixed incompatible rendering of empty hunks in git/unified diffs.
  [#5049](jj-vcs/jj#5049)

* Fixed performance of progress bar rendering when fetching from Git remote.
  [#5057](jj-vcs/jj#5057)

* `jj config path --user` no longer creates new file at the default config path.

* On Windows, workspace paths (printed by `jj root`) no longer use UNC-style
  `\\?\` paths unless necessary.

* On Windows, `jj git clone` now converts local Git remote path to
  slash-separated path.

* `jj resolve` no longer removes the executable bit on resolved files when using
  an external merge tool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

No branches or pull requests

3 participants