Make bundled LFS an option in the installer?

[Dakota Hawkins]

Many of the (great) new GFW features we've gotten recently have also
gotten their own installer options, so why not LFS?

While I think bundling it is great for a lot of users, it irks me
personally a little bit because I have preferred to stay on the latest
and greatest of both GFW and LFS.

I have also contributed a couple of small things to LFS and maintain a
build directory for it where I have been able to swap the LFS
installer's PATH entry with my built copy pretty easily, but now look
out! Player 3 has entered the game!

Both GFW and LFS are updated pretty frequently (LFS a few hours ago!)
but I'm not sure it's reasonable to expect you to turn around a new
GFW patch installer every time LFS has a patch release. Because of
that, I'd much prefer the option to forego the bundled version and
keep both installs on the bleeding edge.

-Dakota

[Johannes Schindelin]

Hi Dakota,

I can understand that reasoning.

Would you mind taking a crack at this new option? We have documented how
to make a custom installer here:

https://github.com/git-for-windows/git/wiki/Making-an-installer

Once you have run through this once, you can then edit
/usr/src/build-extra/installer/install.iss in the SDK, following
https://github.com/git-for-windows/build-extra/commit/c35de0a2132 for
example.

Or you could make it an option in the first page, in which case you want
to add an option to the [Components] section similar to ext\shellhere (but
of cours you would delete git-lfs.exe and unset the config similar to the
"Adapt core.autocrlf" part of the CurStepChanged function.

Please let me know how it goes!
Johannes

[Dakota Hawkins]

Sure, I'll give it a try. Thanks for pointing me in the right direction!

[Dakota Hawkins]

How many "failed retrieving file 'blah' from repo.msys2.org :
Connection timed out after 10000 milliseconds" errors should I be
getting during the SDK install? Lots?

[Dakota Hawkins]

Looks like a problem at least one other person has had in the last
week: https://github.com/Alexpux/MSYS2-packages/issues/849. Checked on
their IRC and most people can connect/tracert fine, mine fails at the
last step, so I may be dead in the water here.

[Dakota Hawkins]

I was able to install the SDK successfully by connecting through my
phone. Building an installer works fine.

I have questions about the config. Presumably, if LFS were optional in
future installers, merely having the option unchecked should not
remove the 'filter.lfs' section from the user's .gitconfig file (since
LFS may be installed and in-use separately).

Do you have any ideas about a reasonable way to handle this? It
appears the other options just go ahead and mess with your config. Is
there a way to remove the exe and sidestep the part of the LFS package
that messes with your config?

I'll admit I'm not familiar with Inno Setup, but I'm not positive
there's an elegant solution. On the one hand, you may destroy
configuration information the user needs to preserve. On the other
hand, you may leave LFS stuff in the user's config that is no longer
required. I'm not sure it's always going to be possible to make the
right choice.

Thoughts?

[Johannes Schindelin]

Hi Dakota,

On Sun, 2 Apr 2017, Dakota Hawkins wrote:

> I was able to install the SDK successfully by connecting through my
> phone. Building an installer works fine.

Good. Just in case, I adjusted the build definitions I use for continuous
testing (where I need to keep several VMs synchronized with the SDK and I
use Git repositories) so that the updates to the Git repositories
mirroring the current state of the 32-bit and the 64-bit Git for Windows
SDK are also pushed to https://github.com/git-for-windows/git-sdk-32 and
https://github.com/git-for-windows/git-sdk-64, respectively.

> I have questions about the config. Presumably, if LFS were optional in
> future installers, merely having the option unchecked should not
> remove the 'filter.lfs' section from the user's .gitconfig file (since
> LFS may be installed and in-use separately).

I would actually assume that we *should* remove the `filter.lfs` section
when removing the `git-lfs.exe` file due to an unchecked box: that section
lives in C:\Program Files\Git\mingw64\etc\gitconfig (or should live there,
I still have to fix that).

So if Git for Windows does not come with Git LFS, the system-wide
configuration makes no sense either. Instead, we should expect individual
users to install git-lfs.exe as needed, then, and also to take care of the
configuration in their ~/.gitconfig.

> Do you have any ideas about a reasonable way to handle this? It
> appears the other options just go ahead and mess with your config.

That is what I would suggest here, too ;-)

> Is there a way to remove the exe and sidestep the part of the LFS
> package that messes with your config?

Yes, we could just leave the config as-is, and have the users deal with
nasty error messages that Git LFS was not found when cloning an LFS-aware
repository.

I think we can do better, though, by removing the `filter.lfs` section
when we remove the `git-lfs.exe` anyway.

> I'll admit I'm not familiar with Inno Setup, but I'm not positive
> there's an elegant solution.

Well, as far as Pascal can ever be elegant :-)

I have to admit that most of my knowledge about InnoSetup comes from `git
grep`ing our InnoSetup configuration files and guessing what the code
does, followed by lots of trial and error.

> On the one hand, you may destroy configuration information the user
> needs to preserve.

Users are not supposed to be able to write to C:\Program
Files\Git\mingw64\etc\gitconfig. I consider that file managed by Git for
Windows, and subject to replacement by future Git for Windows versions.

> On the other hand, you may leave LFS stuff in the user's config that is
> no longer required. I'm not sure it's always going to be possible to
> make the right choice.

The real problem with leaving the config in place arises when a user wants
to clone an LFS-aware repository *without* pulling down gigabytes of data.
In that case, the user really wants to have those placeholder files. And
if the configuration is still in place (with no way to override, because
~/.gitconfig cannot unset what is defined in the system-wide config), Git
will not do what the user expects but complain that `git-lfs.exe` could
not be executed.

I'd rather remove the configuration for that reason, to improve the user
experience of an unchecked "Enable Git-LFS" checkbox.

Please note that I will have to release Git for Windows v2.12.2(2) really
soon now, as there is a critical bug
(https://github.com/git-for-windows/git/issues/1111). All I need to do
before that release is to ensure that the `filter.lfs` section is actually
kept when we want to enable Git LFS support (at the moment, due to a bug,
the `filter.lfs` section is simply thrown away after it was diligently
copied in place). Hopefully tomorrow.

Ciao,
Johannes

[Dakota Hawkins]

I see, I was confused. The installer sets/unsets settings in
C:\Program Files\Git\mingw64\etc\gitconfig, but leaves ~/.gitconfig
alone.

I was worried about the installer removing `filter.lfs` from
~/.gitconfig, because I did not remember that there was a system
config file.

That was especially dumb of me because I install as an admin user that
is not my normal (non-admin) user, so of course that can't be how it
works.

[Johannes Schindelin]

Hi Dakota,

On Mon, 3 Apr 2017, Dakota Hawkins wrote:

> I see, I was confused. The installer sets/unsets settings in
> C:\Program Files\Git\mingw64\etc\gitconfig, but leaves ~/.gitconfig
> alone.

Exactly.

> I was worried about the installer removing `filter.lfs` from
> ~/.gitconfig, because I did not remember that there was a system
> config file.
>
> That was especially dumb of me because I install as an admin user that
> is not my normal (non-admin) user, so of course that can't be how it
> works.

Don't be so hard on yourself. The only way to never be wrong is to never
say anything in the first place.

For what it is worth, I need to work on the Git LFS config anyway, as
there is a bug where the filter.lfs section does not even reach the end
user because the mingw64\etc\gitconfig file is simply deleted.

This, plus a rather serious crash when running `git status` in large
repositories, is reason enough to publish Git for Windows v2.12.2(2), so I
need this rather soon and therefore I will work on this myself. I hope you
don't mind!

Ciao,
Johannes

[Dakota Hawkins]

I don't mind at all, it would have taken me until the weekend at least
to get back to it, I'm just glad to have the toggle!

Thanks again!

Dakota

[Dakota Hawkins]

I tested this with the new option checked, and a "which git-lfs" from
git bash is finding my old system install ('/c/Program Files/Git
LFS/git-lfs') instead of the packaged version.

If I downgrade to 2.12.2 it finds '/mingw64/bin/git-lfs', I would have
expected the 2.12.2.2 install to find the packaged version as well.

I don't see a difference in my system PATH, so I think moving the
git-lfs exe in the new installer keeps it from being found first. This
should mean that the filters (properly configured to use 'git-lfs'
instead of 'git lfs' will find whatever ancient LFS install you happen
to have lying around (!!!) instead of the packaged version.

Testing a little, I have found that with 2.12.2.2:
* `git-lfs version` and `git lfs version` report different versions:
the former uses the system version and the latter uses the packaged
version
* Moving git ('C:\Program Files\Git\cmd') ahead of LFS in my system
path has no effect

IMO you should consider the move in
https://github.com/git-for-windows/build-extra/commit/5f7d44728c089694f4ef1cc3da03f15e35cd5ecc
since LFS is designed go be called separately (`git-lfs`) to avoid the
extra git process (especially on Windows and especially by the
filters) as well as through git (`git lfs`).

I'll see if I can work up a PR for that in the interim.

Thanks again,

Dakota

[Dakota Hawkins]

Note that the commit to move git-lfs in the first place didn't hit the
path in 'mingw-w64-git-lfs/git-lfs.install' so that presumably needs
to be fixed if you don't intend to revert it.

I have tried to revert the move here:
https://github.com/dakotahawkins/build-extra/commit/61c7d1b31e9531b32768845d20f951b027dfda5c

I do not know what I'm missing, but built installers don't want to put
lfs back in the bin folder. My best guess is that there's something
I'm supposed to do with pacman for it and I haven't discovered what
that thing is -- maybe it's pulling down the GFW mirrored package or
something? I can't tell.

Thanks,

Dakota

[Dakota Hawkins]

Nothing like hitting "send" to immediately fix your own problem...

I had to bump the package versions to "3" and then get it into my
local pacman repo/cache (I'm still not sure what thing I did caused
this to happen, but the installer file list looks like it's going to
do what I want now.

PR up: https://github.com/git-for-windows/build-extra/pull/141

[Johannes Schindelin]

Hi Dakota,

On Thu, 6 Apr 2017, Dakota Hawkins wrote:

> I don't mind at all, it would have taken me until the weekend at least
> to get back to it, I'm just glad to have the toggle!

Good, I was slightly worried that my simply going ahead would irk you, as
you had already spent time on it, and I hate to waste anybody's time.

I also just realized that I totally forgot to reply to you with a link to
the change that actually made it into Git for Windows v2.12.2(2):

https://github.com/git-for-windows/build-extra/commit/16975a72fd3

Hopefully we will have a lot more of these fruitful conversations in the
future!

Ciao,
Johannes

[Dakota Hawkins]

Any chance you could help me figure out what I did to make the SDK
installer build pick up my changes?

I think something got pacman to pull in my "upgrade" to the package
once I bumped the package number to 3, but I was kind-of trying
multiple pacman commands from the Wiki and once it worked I wasn't
sure why.

If it's possible I can add what you tell me to the documentation, for
a case where packages don't need to be built from source but
re-packaged (terminology?) for an installer change.

Thanks!

Dakota

[Johannes Schindelin]

Hi Dakota,

On Fri, 7 Apr 2017, Dakota Hawkins wrote:

> Any chance you could help me figure out what I did to make the SDK
> installer build pick up my changes?

Hopefully ;-)

> I think something got pacman to pull in my "upgrade" to the package once
> I bumped the package number to 3, but I was kind-of trying multiple
> pacman commands from the Wiki and once it worked I wasn't sure why.

What I usually do is to call

makepkg-mingw -s

in the appropriate directory to build the package, and

pacman -U <file>

to install it.

Actually, I lied... ;-) What I *really* do is to call build-extra's
`please.sh build git-lfs` and then `please.sh install git-lfs`. That would
only work if you had C:\git-sdk-32 *and* C:\git-sdk-64 fully populated.

That please.sh script is my work horse when maintaining Git for Windows.
It not only automates tons of things, it is also intended to document what
I learned, i.e. a living cheat sheet.

> If it's possible I can add what you tell me to the documentation, for
> a case where packages don't need to be built from source but
> re-packaged (terminology?) for an installer change.

Sure, that would be nice!

Thanks,
Johannes