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

eldev doctor check for tar on MS-Windows #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikappaki
Copy link
Contributor

Hi,

could you please consider patch to add a new doctor check for the compatibility of tar on MS-Windows. It addresses #50.

It returns the following warn when it can't use tar

Is the tar executable found compatible with Eldev on MS-Windows? NO

The tar executable found at `c:/Program Files/Git/usr/bin/tar.exe` is not
compatible with Eldev, please use the one provided with MS-Windows (typically
located at `c:\Windows\system32\tar.exe`), by either rearranging the entries
in the PATH environment variable to pick that up first, or by setting the
`eldev-tar-executable' in `Eldev-local` to something like

(setf eldev-tar-executable "C:/Windows/system32/tar.exe").

Ran 1 doctest, it generated a warning

I have also added a test.

Thanks

@doublep
Copy link
Collaborator

doublep commented Jan 6, 2023

The tar executable found at c:/Program Files/Git/usr/bin/tar.exe is not compatible with Eldev

This largely reads as "Eldev sucks". I would rephrase this to mention what's wrong about it, i.e. mishandling of drive letters in absolute Windows paths.

More importantly, is there no way to convince GNU tar to behave sanely? E.g. an option, an environment variable, some sort of escaping in paths? Eldev already has a shitton of workarounds everywhere, why not add another one? Alternatively, maybe we could use a non-absolute path for tarring on Windows only if possible (i.e. if on the same drive).

@ikappaki
Copy link
Contributor Author

ikappaki commented Jan 8, 2023

The tar executable found at c:/Program Files/Git/usr/bin/tar.exe is not compatible with Eldev

This largely reads as "Eldev sucks". I would rephrase this to mention what's wrong about it, i.e. mishandling of drive letters in absolute Windows paths.

Sure, this is why code reviews are so important :)

More importantly, is there no way to convince GNU tar to behave sanely? E.g. an option, an environment variable, some sort of escaping in paths? Eldev already has a shitton of workarounds everywhere, why not add another one? Alternatively, maybe we could use a non-absolute path for tarring on Windows only if possible (i.e. if on the same drive).

There's this --force-local flag for GNU tar that appears to do the trick.

Let me try to summarise the problem to put any prospective solution into perspective.

Problem: It is common for MS-Windows program to have a (graphical) installer that installs programs into a directory that the user should not be concerned about. Usually, this directory's bin folder is placed at the start of the PATH environment variable (without even the user realizing), so that he program can be invoked by either clicking the program's icon or typing the program names on the command line. Some programs, such as git for windows, are primarily developed on Linux and only ported on MS-Window afterwards. They may have additional binary tools dependencies that usually bundled with, and installed alongside the program. These binary dependencies may not be 100% compatible with MS-Windows but they are most likely to work flawlessly for at least the range of operations required by the main program. In our example case, git for windows comes with a ported version of GNU tar that does not work out for the box on MS-Windows and while it is placed at the front of the PATH, courtesy of the git for windows installation program, is causing Eldev to pick it up in favor of the standard battle tested tar which comes with MS-Windows.

Possible options to workaround both the general and specific GNU tar issue

  1. [General solution] Prefer the battle tested tar program that comes with MS-Windows over any other ported tar version that was installed as a side effect of some other unrelated program's installation. This can be perhaps done by having eldev-tar-executable fn find $WINDIR/system32/tar.exe unless eldev-tar-executable is set explicitly by the user. The previous discussion on this solution suggested to use the absolute c:\Windows\System32\tar.exe path which you advised is a terrible idea (perhaps using the WINDIR env variable makes it more acceptable?).
  2. [General solution] Have eldev doctor check whether the current tar executable found first in PATH is working on MS-Windows, if not, advise user to rejiggle the PATH order so that the windows tar program comes first.
  3. [Gnu tar specific solution] A naive implementation would be to have the eldev-tar-executable fn to check whether the first tar found in PATH is GNU tar (perhaps by executing tar --version), and, if so, set a new var flag to indicate the same . We then update every reference of this fn or eldev-tar-executable variable to also check about whether the new flag has been set, and if so, invoke tar with the --force-local option. It would be requiring some effort to write a test for this, but should be achievable.

What are you thoughts? As I said in one of my previous comments some time ago, I'm in favor of solution#1 since this is most likely to work under every circumstance, and it is most unlikely that the user has consciously installed a tar port on their MS-Windows machine (there is high chance this was done as a side effect as part of an unrelated program's support operation).

Thanks

@doublep
Copy link
Collaborator

doublep commented Jan 9, 2023

Thanks for the detailed analysis, now I understand the issue much better. However, I still don't feel like overriding what PATH says and (semi-)hardcoding which tar is to be used.

I suggest going with 2 + 3. Implement whatever hacks are necessary to make Eldev work even with GNU tar (3). But still add a check to the doctor (2) that would complain about GNU tar being in path: even if Eldev itself is able to cope with it now, it would potentially be problematic for general use on Windows.

Also:

It would be requiring some effort to write a test for this, but should be achievable.

Tests are nice to have, but not an absolute must. In this case a test can be skipped as long as you say that it works fine on your machine.

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

Successfully merging this pull request may close these issues.

None yet

2 participants