Re: [PATCH 2/2] Allow passing pipes for input pipes to diff --no-index

Hi Taylor,

On Fri, 18 Sep 2020 at 10:36, Taylor Blau <me@ttaylorr.com> wrote:

Bash, ksh, zsh and likely any more moden shell. Other programming languages also setup such pipes. It's much cleaner than creating temp files and cleaning them up and in some cases faster too (I've ran diff's like this over GB's of test data, it's very handy to remove known patterns that would cause needless diffs).

I'll admit the comment was written first and is a bit naive - i'll rephrase that. Yes you don't want to block on pipes like if you run a "grep -R" on a subtree that has fifos - but as I coded this I realized the obvious: git tracks symlinks name so the real bugger would be to detect one as a pipe and try reading it instead or calling readlink().

The whole block is under lstat/S_ISLNK (see previous chunk), so the path provided to us was a symlink.

Initially I looked at what differentiate these - mainly, stat() st_dev
- but that struct is os-specific, you'd want to check major(st_dev) ==
0 (at least on linux) and even if we knew how each os behaves, the
code isn't portable and would be a pain to support. Gnu's difftools
have very incomplete historical source code in git but there's
indications they have gotten rid of it too.

So what I'm doing instead is trying to resolve the link and see if the destination exists (a clear no). Luckily strbuf_realpath does the heavy lifting and leaves me with a real path to the file the symlink points to (especially useful for relative links), which is bogus for the special pipes we're interested in.

Then the block right after (not shown) do a stat() on the initial name and return whenever it's a fifo or not (if it is, but the link is broken, we know it's a special device).

Now you mention it, maybe I could do that stat first, rule this out from the beginning... less work for the general case.

*untested*:

    if (!lstat(name, &st)) {
        if (!S_ISLNK(st.st_mode))
            return(0);
        if (!stat(name, &st)) {
            if (!S_ISFIFO(st.st_mode))
                return(0);

            /* We have a symlink that points to a pipe. If it's resolved
             * target doesn't really exist we can safely assume it's a
             * special file and use it */
            struct strbuf sb = STRBUF_INIT;
            strbuf_realpath(&sb, name, 0);
            /* We're abusing strbuf_realpath here, it may append
             * pipe:[NNNNNNNNN] to an abs path */
            if (stat(sb.buf, &st))
                return(1); /* stat failed, special one */
        }
    }
    return(0);

TL;DR - the conditions we need:

- lstat(name) == 0  // name exists
- islink(lstat(name))  // name is a symlink
- stat(name) == 0  // target of name is reachable
- isfifo(stat(name))  // Target of name is a fifo
- stat(realpath(readlink(name))) != 0  // Although we can reach it,
name's destination doesn't actually exist.

BTW is st/sb too confusing ? I took examples elsewhere in the code, I can rename them if it's easier to read.

Thanks for the tip - indeed I think I ran the testsuite directly with back, but the make test failed.

Cleanup, no pipelines (I read too fast / assumed last command was ok) - will do!

Precautions/portability. The file names are somewhat dynamic (at least the fd part...) this is to be sure I capture the names of the pipes that will be used (assuming the fd's will be reallocated in the same order which I think is fairly safe). An alternative is to sed "actual" to remove known variables, but then I hope it would be reliable (and can I use sed -r?). IIRC earlier versions of bash or on some systems a temp file could be used for these - although it defeats the purpose it's not a reason to fail....

I cannot develop this on other systems but I tested the pipe names on Windows and Sunos, and also using ksh and zsh on Linux (zsh uses /proc directly, kss uses lower fd's which means it can easily clash with scripts if you don't use named fd's, but not our problem....)

Thanks,

Thomas