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

Could you please package the 'rev' command with Git for Windows #2497

Closed
NBardelot opened this issue Jan 29, 2020 · 17 comments
Closed

Could you please package the 'rev' command with Git for Windows #2497

NBardelot opened this issue Jan 29, 2020 · 17 comments

Comments

@NBardelot
Copy link

All in the title. It's a common and useful command.

@dscho
Copy link
Member

dscho commented Jan 29, 2020

Git for Windows is open source, if you so wish that rev is included, and have convincing arguments, and if it does not bloat Git for Windows' installer, you can make it happen.

@NBardelot
Copy link
Author

Thanks for your fast reply.

I currently commit and report on other Open Source projects. I understand why as a maintainer you tell people to join in (and that's a good thing), but the time and complexity of joining a new project, learning architecture/integration/tests/policies/best practices and norms is costly. If the task is easy, and several users want the feature, simply asking is often more efficient.

If the task of adding 'rev' is easy, I'd be really glad if someone with good knowledge of the project could do it. If it is not, I'd understand that you refuse the feature request.

@NBardelot NBardelot changed the title Please package the 'rev' command with Git for Windows Could you please package the 'rev' command with Git for Windows Jan 29, 2020
@dscho
Copy link
Member

dscho commented Jan 29, 2020

If the task of adding 'rev' is easy, [...]

As long as there is little in the way of documentation what it is, where to find it, what needs to be included to make it work, how much it adds to the payload of the Git for Windows installer, it won't be easy for anybody else.

@rimrul
Copy link
Member

rimrul commented Jan 30, 2020

Let me shine some light on this:

Here's the man page for rev.

The rev utility copies the specified files to standard output, reversing the order of characters in every line. If no files are specified, standard input is read.

It's part of util-linux and the good news is theres already a util-linux MSys2 package.

On the other hand I dont' feel that rev should have strong MSys2 dependencies and a MinGW-w64 package might help with startup time and assembly size.

@dscho
Copy link
Member

dscho commented Jan 30, 2020

Silly question: what does rev have to do with Git? In other words: why should Git for Windows ship it?

Sounds more like a task for a full MSYS2 to me.

@NBardelot
Copy link
Author

NBardelot commented Jan 30, 2020

@dscho

Your question helps me understand that I might be asking the feature to the wrong project.

Here's what I see in a typical Git for Windows installation:

> (cd "C:\Program Files\Git\mingw64\bin" && find . -name '*.exe')
./acountry.exe
./adig.exe
./ahost.exe
./antiword.exe
./blocked-file-util.exe
./bunzip2.exe
./bzcat.exe
./bzip2.exe
./bzip2recover.exe
./certtool.exe
./connect.exe
./corpus2array.exe
./create-shortcut.exe
./curl.exe
./edit_test.exe
./edit_test_dll.exe
./envsubst.exe
./gettext.exe
./git-askyesno.exe
./git-credential-helper-selector.exe
./git-lfs.exe
./git-receive-pack.exe
./git-upload-archive.exe
./git-upload-pack.exe
./git.exe
./lzmadec.exe
./lzmainfo.exe
./odt2txt.exe
./openssl.exe
./p11tool.exe
./pdftotext.exe
./pkcs1-conv.exe
./proxy-lookup.exe
./psktool.exe
./recode-sr-latin.exe
./sexp-conv.exe
./srptool.exe
./tclsh.exe
./tclsh86.exe
./unxz.exe
./WhoUses.exe
./wintoast.exe
./wish.exe
./wish86.exe
./x86_64-w64-mingw32-agrep.exe
./x86_64-w64-mingw32-deflatehd.exe
./x86_64-w64-mingw32-inflatehd.exe
./xmlwf.exe
./xz.exe
./xzcat.exe
./xzdec.exe
./zipcmp.exe
./zipmerge.exe
./ziptool.exe

But I don't know how MINGW64 dependencies are managed in Git for Windows. My goal is to get the rev.exe utility as well as those other ones.

@dscho
Copy link
Member

dscho commented Jan 30, 2020

I don't know how MINGW64 dependencies are managed in Git for Windows.

Git for Windows is essentially a subset of MSYS2, with a couple customized (or added) components. See https://github.com/git-for-windows/git/wiki/Package-management for more on this.

See also https://github.com/git-for-windows/git/wiki/Technical-overview and https://github.com/git-for-windows/git/wiki/Making-an-installer.

@dscho
Copy link
Member

dscho commented Jan 30, 2020

@NBardelot I had another look, and it seems that the load would not be too much:

$ ls -lh /usr/bin/rev.exe
-rwxr-xr-x 1 EUROPE+johasc 4096 13K Jul  9  2019 /usr/bin/rev.exe*

$ ldd /usr/bin/rev.exe
        ntdll.dll => /c/windows/SYSTEM32/ntdll.dll (0x7ff99fc20000)
        KERNEL32.DLL => /c/windows/System32/KERNEL32.DLL (0x7ff99eba0000)
        KERNELBASE.dll => /c/windows/System32/KERNELBASE.dll (0x7ff99ced0000)
        msys-2.0.dll => /usr/bin/msys-2.0.dll (0x180040000)
        msys-intl-8.dll => /usr/bin/msys-intl-8.dll (0x17cc50000)
        msys-iconv-2.dll => /usr/bin/msys-iconv-2.dll (0x5603f0000)

(Those msys-*.dll files are already included, being dependencies of cmp.exe, diff.exe and grep.exe.)

As @rimrul pointed out, rev is included in the package util-linux:

$ pacman -Qo /usr/bin/rev.exe
/usr/bin/rev.exe is owned by util-linux 2.34-1

If you look through the output of pacman -Ql util-linux, you will find that it also includes getopt.exe, something we ship in Git for Windows specifically to support git flow: git-for-windows/build-extra@920c19b

@NBardelot If you imitate the getopt.exe part of that commit, you will get your wish (in a Git for Windows SDK, call sdk cd build-extra to get to the directory containing make-file-list.sh quickly, and use sdk build installer after making the changes to verify that they do what you want).

To get this include in Git for Windows, all you need is a convincing commit message. Convincing as in: mentioning how small the tool is, and then also trying to make a case for how useful it is (you will have to have a strong argument there, as indicated by my lack of familiarity with that tool, and I am a shell script power user). Then open a PR, see it merged, and celebrate. 🍰 🍾

@chrisbra
Copy link

I have never really had the need for rev in a linux environment so I don't exactly know what your use case here is. So how about using this:

chrisbra@hostname ~/git-bash $ cat rev.awk
#!/usr/bin/awk -f
BEGIN {FS="";}
{
        rev="";
        for (i=1; i<=NF; i++)
                rev = $i rev;
        print rev
}
chrisbra@hostname ~/git-bash $ echo -e "123\n456\n789" |  ./rev.awk
321
654
987

@NBardelot
Copy link
Author

NBardelot commented Jan 30, 2020

@chrisbra yes I use this kind of workaround for the moment. But using awk to reverse a string is a bit overkill. Plus, if the script comes from someone working on Linux, they usually won't bother adapting it like that (as they can use rev as a quite standard tool).

A typical use of rev is to get the N last elements of a path (on a filesystem or in a url). Using cut with a pattern rev-cut-rev in order to cut from the end : echo -n '1/2/3/4' | rev | cut -d '/' -f1-2 | rev to get 3/4. There is no simple awk syntax (without parsing character by character like your workaround) to obtain the same feature.

@dscho
Copy link
Member

dscho commented Jan 30, 2020

using awk to reverse a string is a bit overkill.

Shipping rev.exe to millions of Git for Windows users for no obvious gain is even more overkill.

if the script comes from someone working on Linux, they usually won't bother adapting it like that

Repeat after me: Git for Windows is not a replacement for Linux. Git for Windows is not a replacement for Linux. Git for Windows is not a replacement for Linux. Git for Windows is not a replacement for Linux.

The closest you have if you want to run scripts on Windows that run on Linux software, as unmodified as possible, is Cygwin. It is rather complete, the only downside is that you pay for that unwillingness to adjust your scripts to Windows by suffering the speed penalty of the POSIX emulation layer.

Granted, there are a lot of shell scripts that work out of the box with Git for Windows.

That does not mean that I have to support those use cases, though. And given that users like you are happy to ask yet shy to deliver, the support burden would really stick with me, and I'll just not do that.

A typical use of rev is to get the N last elements of a path

So you spawn a whopping three processes instead of a single one, wasting time by reversing characters twice? This sounds quite a bit like "taking the scenic route" (or, if you want, "going around your elbow to get to your ear").

Wouldn't you rather need something more along the lines of echo -n 1/2/3/4 | sed -En 's@^(.*/|)([^/]*/)@\2@p'? It'll be much faster and much more portable.

I am coming more and more to the conclusion that this wish is not worth the effort already expended in this ticket, and given that you, @NBardelot, did not exactly jump on my earlier comment outlining what needs to be done, I start to realize that you came to the same conclusion.

In the least, I have not seen anything remotely resembling that convincing argument I would require to accept a change to include that tool: to the contrary, so far, it would seem rather pointless to include it.

@dscho
Copy link
Member

dscho commented Jan 30, 2020

There is no simple awk syntax (without parsing character by character like your workaround) to obtain the same feature.

Okay, so I spent 15 minutes of my life learning me some awk to correct someone who is wrong on the internet:

echo 1/2/3/4 |
awk '{split($1, x, "/"); if (length(x) > 1) print x[length(x) - 1] "/" x[length(x)]}'

So yes, it can be done in awk. It can also be done in perl, shorter, of course:

$ echo 1/2/3/4 | perl -ne '/([^\/]*\/[^\/]*)$/ && print $1'

@NBardelot
Copy link
Author

NBardelot commented Jan 31, 2020

If being right matters so much to you, great, be right on the Internet. I was asking politely for a feature I think is useful for git users in a git context, but if that means to interact with some obnoxious maintainer, it wasn't even worth each others time. Let's close this ticket.

@dscho
Copy link
Member

dscho commented Jan 31, 2020

If being right matters so much to you, great, be right on the Internet.

Sorry, I thought you would appreciate learning something new, and yes, I pulled your leg a little because you stated your opinion as if it were fact, and it was in fact untrue.

You said that there is no simple awk syntax to chomp off the last two path components in a list of paths. And I just proved to you that there is one.

I apologize if I mistook your bold statement for an invitation to be taught a new trick.

I was asking politely for a feature I think is useful for git users in a git context,

As I stated multiple times, I am not convinced that the rev utility is useful for anything Git-specific in particular. In fact, I am not convinced at all that it helps with any task that I would write a shell script for. And keep in mind that I am a rather prolific shell scripter. It's not like I have no idea what I am talking about.

but if that means to interact with some obnoxious maintainer, it wasn't even worth each others time.

It surprises me that you think that my behavior was obnoxious. I asked you multiple times to come up with a convincing use case. That is, a use case where rev.exe is required, and where it would not be much easier to write a Perl, sed or, yes, awk invocation instead.

The work I reported in this comment was actually work I would have expected of a contributor such as yourself. Yet it looks like you were very happy to just ask and leave all the doing to me. So yes, I am a bit disappointed here. This is not a sustainable way to do open source, for millions of users to merely mention their wishes and then expect the single maintainer to implement those wishes. Therefore, I am respectfully okay with this:

Let's close this ticket.

@dscho
Copy link
Member

dscho commented Jan 31, 2020

to chomp off the last two path components

My mistake: I meant to say "to chomp off and display the last two path components"

@NBardelot
Copy link
Author

NBardelot commented Jan 31, 2020

Being obnoxious is not about technical knowledge, even though your notion of ”simple“ is not mine at all (3 chars command, very small binary, that's why it exists for this job, Unix-style, even if git is not Unix). Thanks to your technical skills the project exists, works, and I use it. For that you deserve to be thanked without doubt.

Being obnoxious is about bashing me while I try to interact with the project in a constructive manner; setting an ultimatum (I have a job, and no I won't respond to your multiple questions in the timely manner you want to impose); reinterpreting my answer to someone else the less charitable way, to drive your point and your point only; and being condescending. If you choose to be that way, good for you. But if you don't see that it is the way you behave, let this be my feedback.

I was coming, as a user not familiar with the inner parts of the project, just asking for a feature. You could have stopped at the part where you said ”if you want it, here's a starting point, and you'll have to argue for your PR”. I was OK with that, in fact I had 👍 that, and I was ready to take a look. Then, you're the main commiter, and maintainer of the project. You deal with your users how you see fit.

@dscho
Copy link
Member

dscho commented Jan 31, 2020

I was coming, as a user not familiar with the inner parts of the project, just asking for a feature. You could have stopped at the part where you said ”if you want it, here's a starting point, and you'll have to argue for your PR”. I was OK with that, in fact I had 👍 that, and I was ready to take a look.

FWIW it was totally unclear to me that you were ready to take a look. To me, what you wrote sounded totally the opposite: that you would never ever take a look. In the end, it seems like that happens anyway, even if you blame me for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants