Git: "Directory" parameter fails to follow paths with spaces

[mirh]

PortableGit.7z.exe -y -InstallPath="C:\test1" -Directory="C:\test1" works
PortableGit.7z.exe -y -InstallPath="C:\test 1" -Directory="C:\test 1" extract files, but returns a "Error during execution C:\test 1\git-bash.exe --no-needs-console --hide --no-cd --command=post-install.bat. Cannot find the file specified" error.

According to process monitor, QueryDirectory gets lost to "C:\test.*". so I guess there are some quotes missing in the source code of the 7z sfx extractor (that I could find nowhere though).

[shiftkey]

Here's the source for how the PortableGit extractor is created - the path to the installer isn't specified with the RunProgram parameter.

[mirh]

My mildly educated guess is that the \ is escaped from the quotes in RunProgram.
Or perhaps the point is that there should be "%%S" like in InstallPath?

Also, "This script cannot handle spaces in $SCRIPT_PATH" doesn't look good.

[dscho]

In the meantime, I switched to a new SFX component based on the offical 7-Zip SFX. One reason was to avoid being stuck with a no-longer-maintained SFX (the original website, http://7zsfx.info is no longer registered and there is no information what happened to the Subversion repository hosted at http://svn.7zsfx.info), another reason was to allow contributors to fix bugs.

One fallout is that the command-line options mentioned in the original report will no longer work at all.

But all is not lost: a fix to this ticket only needs a dedicated contributor with a little time and drive: simply clone https://github.com/git-for-windows/7-Zip, open CPP\7zip\Bundles\SFXSetup\SFXSetup.sln in Visual Studio 2017 (the free Community Edition should work just fine), switch the mode to ReleaseD and start hacking.

@mirh please give it a try.

[mirh]

I'd love to, but atm there's a big problem with it
https://help.github.com/articles/error-remote-head-refers-to-nonexistent-ref-unable-to-checkout/

[dscho]

I'd love to, but atm there's a big problem with it
https://help.github.com/articles/error-remote-head-refers-to-nonexistent-ref-unable-to-checkout/

Fixed. I pushed only one branch originally, and that used to work correctly (it almost did, I had to push a second branch to be able to select another default branch, then simply changed back and deleted the temporary branch).

[mirh]

Mhh, after reading a lot of stuff... Seems like there should be already some kind of thing like that?
It's missing from sfxsetup.. So I guess it should "just" be ported from sfxcon or archivecommandline? I don't consider myself worthy tbh then.

In other news though, it seems like the original 7zsfx has been updated up to an exact year ago. And you should also update .gitignore with *.VC.db, *.VC.VC.opendb, *.user files.

[dscho]

Sorry, I did not want to give you the impression that I am less than completely overwhelmed with work and that I would not actually need your help here. I do need your help here, badly.

[mirh]

Ditto 🙃🙃
Guess like google's depot_tools will have to wait some other time to support spaces.

[dscho]

On Fri, 7 Apr 2017, mirh wrote:

Ditto 🙃🙃

Pity.

Guess like google's depot_tools will have to wait some other time to support spaces.

Actually, with the switch to the new SFX, Google's depot_tools may be
completely broken now, as no command line options are supported. So they
will now be broken even for paths without spaces.

Maybe that's enough incentive to jump in and help?

[mirh]

Perhaps for some google's employee :s

[mirh]

Wat?
EDIT: I'll _edit_ here to avoid drama and reply the other thread
"If there is no [...], I need to close them."
No? What's the reason? Everybody would have better understood and call it a day if you just had explained that. No need to waste everybody precious time reading/writing all the rest.

[dscho]

To clarify why this was closed: In an asymmetric open source project like Git for Windows (many, many, _many_ users, Git for Windows v2.20.0(1) was downloaded over 5 _million_ times, see https://www.somsubhra.com/github-release-stats/?username=git-for-windows&repository=git, and less than half a dozen active contributors), we cannot let tickets languish that have no chance of seeing any activity: that would overwhelm the few contributors even more.

And there was more than one indication in this here ticket that nobody involved in the discussion was willing to do more than contribute the discussion, therefore I really _had_ to close it. It was very clear that nothing would be done about this, and I really _had_ to close it so that it was easier to find the tickets that had _some_ activity in them. I am but on human, too, it would just have been too much for me to weed through tons of languishing tickets just to find those that really would benefit from my input.

@mirh I hope this clarifies the reason to you.

I don't mean to complain that there are too few contributors to Git for Windows. I _understand_ that people work on too many things and have to focus on what is _really_ important to them.

Therefore, I understand that this ticket did not see any code being developed. And therefore I closed the ticket, to avoid distracting from those tickets that did see code being developed. That's all.

[mirh]

More than clarify (I already understood the purported point), I'm glad I got a lengthy answer.
But I still don't really see the connection between "no available manpower" and "not worth an entry"
If we were talking about "reasonable expectation nobody could work on it until heath death of universe", well I guess like that could even had its sense.
Otherwise, I don't know.. does it really happen so often that people throw at you moon shots?
And can't you just use tags (such as "long term" or "aspirational goal") like other projects to filter them?

[PhilipOakley]

Hi @mirh

I think dscho was trying to say that (the link between) the "no available manpower" was directly onto a "won't be able to fix any time soon, and next year's not looking good either".

The entry is still here under the 'closed' category, so it can be re-opened should someone step forward with some coding effort.

There maybe a need for an all-encompassing categorisation label for these, but the curation of the labels themselves becomes a problem. Often the real issue is that the users don't always know how to use the Github search to find previous discussions relevant to their issue (I had that problem initially because of the implicit is_open filters and the like), but more importantly the lack of available support effort to span the multiple domains needed for the Git-For-Windows environment (consoles/installers/OSes/...)

[mirh]

Labels are for "knowing developers" eventually.

And anyway, whether you end up sticking a long term one to bugs that remain open or get closed (as I said, I don't see anything wrong with stale open bugs though) that would already look leaps and bounds better than this "we throw it in the same basket of all other solved/wontfix bugs"

[dscho]

If it helps you, just blame it on my "bug tracker dyslexia": if there are too many open tickets, I cannot focus on the important ones, I neglect all of them.

If you don't like that, you could help triage bugs and help push forward stale tickets.

[mirh]

Well, OCD is certainly a fair enough reason to close issues I guess..
Distinguishing actually "completed" ones from those just "shunned", would seem a must to me though

[dscho]

Distinguishing _actually_ "completed" ones from those just "shunned", would seem a must to me though

Okay. I suggest to put the money where your mouth is, and to mark those closed issues you deem "shunned" by adding comments to that extent. I will then add the appropriate labels, and once I am confident that your assessment matches mine, I will add you as a reviewer so that you can add those labels yourself.

[mfriedrich74]

Hi,

To be fair, we all adjust to the tools available to us. If this is the way
the github frontend can be used the easiest and keep the job simple with
fewest clicks, so be it.

Also keep in mind that Status and Resolution are to different attributes of
tickets. Tickets don't disappear when closed, they can be reopened when
things change.

I agree, there several views on the project, the one from user, and those
of the maintainer and contributors. Maybe the solution is less a change on
the projects side, but rather on github (e.g. default filters open issues
versus unresolved issues) to provide the information each individual is
interested in. Without that, it's the maintainer's choice that we have to
respect.

On Fri, Aug 2, 2019, 6:50 AM mirh [e-mail] wrote:

Well, OCD is certainly a fair enough reason to close issues I guess..
Distinguishing actually "completed" ones from those just "shunned",
would seem a must to me though

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/git-for-windows/git/issues/1094?email_source=notifications&email_token=ABZH5SBWQPVEZQL5A5AOOADQCQGPVA5CNFSM4DD667NKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3NNBWA#issuecomment-517656792,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABZH5SGNHEAI2GNR7NOXYELQCQGPVANCNFSM4DD667NA
.

[dscho]

To "un-derail" this ticket, let's focus again on what was asked for (and not yet tackled by any interested contributor).

The feature asked for in this ticket was to re-implement support for -InstallPath. While this is still not implemented, at least now the -o<path> option is implemented, addressing https://github.com/git-for-windows/git/issues/1144.

The actual implementation can be seen here: https://github.com/git-for-windows/7-Zip/blob/dfe38366c6998f87971f89ae6082ee25f7d21028/CPP/7zip/Bundles/SFXSetup/SfxSetup.cpp#L167-L207

In order to get support for -InstallPath in addition to -o, this would have to be refactored, probably by introducing a helper function that will parse the option, probably something like this:

static inline void SkipQuotes(const UString &switches, unsigned &pos, bool &quoted)
{
    while (switches[pos] == L'"') {
        quoted = !quoted;
        pos++;
    }
}

static inline void SkipWhitespace(const UString &switches, unsigned &pos)
{
    while (switches[pos] == L' ' || switches[pos] == L'\t')
        pos++;
}

static inline bool ParseOptionWithValue(UString &switches, const char *optionName, UString &value)
{
    unsigned pos = 0;
    bool quoted = false;

    SkipQuotes(switches, pos, quoted);
    if (switches[pos++] != L'-')
        return false;
    SkipQuotes(switches, pos, quoted);
    unsigned len = strlen(optionName);
    if (memcmp(switches.Ptr(pos), optionName, len * sizeof(wchar_t)))
        return false;
    pos += len;
    SkipQuotes(switches, pos, quoted);
    if (switches[pos] == L'=')
        pos++;
    SkipWhitespace(switches, pos);

    value = L"";
    for (; pos < switches.Len(); pos++)
    {
        wchar_t c = switches[pos];
        if (c == L'"')
        {
             if (!quoted)
                 quoted = true;
             else if (pos + 1 < switches.Len() && switches[pos + 1] == L'"')
             {
                  value += L'"';
                  pos++;
             }
             else
                  quoted = false;
             continue;
         }

         if (!quoted && (c == L' ' || c == L'\t'))
             break;
         value += switches[pos];
     }
     switches = switches.Ptr(pos);

     return true;
}

and then using this as ParseOptionWithValue(switches, L"o", installPath) and also as ParseOptionWithValue(switches, L"InstallPath", installPath).

So: another opportunity for every interested contributor to put their money where their mouth is.