orderfile syntax for git diff ... -O <orderfile>

[jeanremacle]

• I was not able to find an open or closed issue matching what I'm seeing

Setup

• Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

$ git --version --build-options
git version 2.19.1.windows.1
cpu: x86_64
built from commit: 11a3092e18f2201acd53e45aaa006f1601b6c02a
sizeof-long: 4
sizeof-size_t: 8

• Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

$ cmd.exe /c ver

Microsoft Windows [Version 6.3.9600]

• What options did you set as part of the installation? Or did you choose the
  defaults?

# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: VIM
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: OpenSSL
CRLF Option: LFOnly
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Disabled
Enable Builtin Rebase: Disabled
Enable Builtin Stash: Disabled

• Any other interesting things about your environment that might be related
  to the issue you're seeing?

No

Details

• Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

Bash

• What commands did you run to trigger this issue? If you can provide a
  Minimal, Complete, and Verifiable example
  this will help us understand the issue.

git diff --cached --name-only --diff-filter=ACM -O sort_pl.srt

• What did you expect to occur after running these commands?

Output of file names ordered according to order-file passed after the "-O" parameter

• What actually happened instead?

I tried multiple possible syntax for the order file and nothing appeared to have an impact.
I did not find the syntax for the orderfile anywhere.

--> What I'd like to fine is a working example of am orderfile under windows.

Here are a few of them:

---

*.sql
*.pks
*.pkb

---

*sql
*pks
*pkb

---

**.sql
**.pks
**.pkb

---

**/*.sql
**/*.pks
**/*.pkb

---

• If the problem was occurring with a specific repository, can you provide the
  URL to that repository to help us with testing?

Private repository

[dscho]

Private repository

How about creating a tiny public repository for other people to reproduce the issue easily and quickly?

[Osse]

Does the orderfile have Windows style line endings? For me orderfile works fine if the file has Unix line endings. I came here to report it when I saw this issue.

Not sure what the expected behaviour is, but it make sense to me that if Git accepts CRLF in .gitignore and .gitattributes it should for the orderfile as well.

[dscho]

Not sure what the expected behaviour is, but it make sense to me that if Git accepts CRLF in .gitignore and .gitattributes it should for the orderfile as well.

@Osse absolutely.

And even better: this makes for an excellent first Pull Request.

The problem

The file diffcore-order.c contains a function called prepare_order() which reads the order file and parses it: https://github.com/git-for-windows/git/blob/v2.21.0-rc2.windows.1/diffcore-order.c#L11-L58

The parser iterates over the file contents, and in each loop iteration it first tries to look for the end of line, and this is where the culprit lies: it looks only for \n.

Possible solutions

The easy route

In my mind, the easiest fix would be to introduce a function that collapses all CR/LF line endings in an strbuf to LF line endings. Something like

void strbuf_crlf_to_lf(struct strbuf *buf)
{
    char *src = buf->buf, *end = buf->buf + buf->len, *dest = src;

    while (src != end)
        if (src[0] != '\r' || src[1] != '\n')
            *(src++) = *(dest++);
        else
            src++;

    buf->len = dest - buf->buf;
}

This would need to be called directly after reading the contents of the orderfile in prepare_order().

Make prepare_order() CR/LF aware

This might look cleaner from the design perspective, but will be harder to implement (and to verify). The reason for this is that there are a few mentions of \n, and they all assume that this is always the end of the line, when in fact it could be \r. In one case, this assumption is used to look if the line is empty: https://github.com/git-for-windows/git/blob/v2.21.0-rc2.windows.1/diffcore-order.c#L36

But the ep pointer in https://github.com/git-for-windows/git/blob/v2.21.0-rc2.windows.1/diffcore-order.c#L32-L51 really wants to point at the \n, so line 33 is okay, but lines 36 and 41 need to be adjusted. So I could imagine that line 36 would need to look like this:

if (*cp == '\n' || (*cp == '\r' && cp[1] == '\n') || *cp == '#')

and after line 41 (if (*ep == '\n') {) we probably really want to adjust line 42 (*ep = 0;) like this:

if (*ep == '\n') {
    if (ep[-1] == '\r')
        ep[-1] = '\0';
    else
        *ep = '\0';

How to implement/test this?

1. Download and install Git for Windows' SDK
2. Initialize Git: sdk cd git
3. Edit the file: vi diffcore-order.c (or use whatever your favorite editor is instead)
4. Build: make -j5
5. Test: ./git --exec-path=$PWD -C /path/to/your/repository diff -O ...

Once it works, commit it (use the existing non-merge commits as examples to follow, in particular answering the question "why?" in the commit message and leaving the "how?" to the diff), push it to your fork on GitHub and open a Pull Request.

[jeanremacle]

Hello,

Thanks for the info @Osse. It works when I change all CRLF by simple LF.

@dscho, I don't have access to gitlab from my work laptop. I want to try if I can do the change this weekend. A nice challenge for me.

Cheers
Jean

[dscho]

@jeanremacle I'm curious: how's it going?

[dscho]

I don't have access to gitlab from my work laptop.

For the record, you do not need access to GitLab from your work laptop to work on this... Git for Windows' source code is hosted on GitHub...