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

Win installer upgrade #875

Merged
merged 10 commits into from Feb 2, 2016
Merged

Conversation

strich
Copy link
Contributor

@strich strich commented Nov 30, 2015

A new INNO Setup based windows installer. Features:

  • Issue Windows installer is ignoring installation path #819 - The installer will search the PATH environment vars for the location of any actively installed Git instance and use it to find out where to place Git LFS.
  • If Git isn't installed or isn't in the env vars then the installer will simply ask the user to tell it where to install Git LFS.
  • The installer is dual arch and should run on 32/64bit systems. However no effort has been made to actually install the correct git-lfs.exe file yet.
  • It hasn't been setup, but its possible to integrate code signing either in the compile commandline or in the script itself.
  • Issue Git LFS installers should run 'git lfs init' as part of installation #799 - The installer and uninstaller run git lfs install/uninstall for you.

Should also fix:

#define MyAppVersion "1.1.0.0"
#define MyAppPublisher "GitHub, Inc"
#define MyAppURL "https://git-lfs.github.com/"
#define MyAppExeName "Git-LFS-Installer.exe"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of defining this here, can we make it dynamic based on the version? git-lfs-windows-amd64-{#MyAppVersion}.exe. Also, does the version need 4 digits? If we can get away with a MyAppVersion of x.x.x, I think I'd prefer that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep should be easy to do.

…spects existing naming conventions.

Set the existing version number to 3 digits instead of 4.
@technoweenie technoweenie mentioned this pull request Dec 2, 2015
17 tasks
ExpandConstant('{cmd}'),
'/C "for %i in (git.exe) do @echo. %~$PATH:i > "' + TmpFileName + '"',
'', SW_HIDE, ewWaitUntilTerminated, ResultCode
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic because git installers typically setup an alternate shell shortcut, like "Git CMD". But the default %PATH% will not have Git anywhere. Not sure what we can do about that though.

FWIW the shortcut is just "C:\Program Files\Git\git-cmd.exe" --cd-to-home.

@technoweenie
Copy link
Contributor

What do you think about a step that asks for your Git For Windows installation dir? From what I can tell, we'd have to put git-lfs.exe in multiple places. Assuming the default of c:\Program Files\Git:

  • c:\Program Files\Git\cmd - used by Git CMD.
  • c:\Program Files\Git\bin - not sure, seems legit though.
  • c:\Program Files\Git\mingw64\bin - used by Git Bash.

I don't really care about supporting GitHub Desktop or Sourcetree, since both bundle git lfs with their own bundled versions of Git.

Having an input so people can add whatever path they want is important, but I also would like to support the standard Git For Windows installer too.

@strich
Copy link
Contributor Author

strich commented Jan 19, 2016

This installer does already have a step where the user can optionally change the install directory. We simply try to find it for them and make it the default directory.

In my tests only 1 directory requires the lfs executable - c:\Program Files\Git\mingw64\bin. Putting it there works in Git Bash as well as Powershell/cmd.

…rk, the old Git LFS must be uninstalled - This installer will attempt to do this for you silently.

Added support for compiling x86 and x64 binaries into a single 32bit installer.
@strich
Copy link
Contributor Author

strich commented Jan 28, 2016

As per the commit comments, the installer will now install LFS into its own program dir in program files. It will compile both x86 and x64 binaries into a single installer and install the correct one based on the OS arch.

Note that to allow the move to the new file location, we cannoy simply have the LFS dir at the start of the PATH env var as Git will choose to use the git-lfs.exe file in its own local directory prior to looking in PATH.

@technoweenie technoweenie mentioned this pull request Feb 2, 2016
15 tasks
@technoweenie
Copy link
Contributor

Cool, worked great for me 🤘 Thanks so much for hacking on this!

technoweenie added a commit that referenced this pull request Feb 2, 2016
@technoweenie technoweenie merged commit 0640430 into git-lfs:master Feb 2, 2016
@ygoe
Copy link

ygoe commented Mar 23, 2016

#799 is still open, and according to my observation this is correct (the installer does not update the config files). Yet this is mentioned in the description and this is merged. Something went wrong here!

@technoweenie
Copy link
Contributor

Is this a system vs global issue, as you mentioned in #799 (comment) ? The installer should be running the command here.

@technoweenie
Copy link
Contributor

@dg9ngf: The problem is just that git lfs is not installing to something in $PATH. I posted a new issue with the specific issues in: #1120. Please continue the discussion there, it's very easy to lose track of discussion on closed items. Thanks!

@ygoe
Copy link

ygoe commented Apr 3, 2016

Sounds reasonable. I just can't see this as a regular user of the installer. I've subscribed to the other issue. BTW, I didn't change the default install path, so this should hit everybody.

chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 31, 2022
In PR git-lfs#875 in 2016 the Windows installer was changed from NSIS
to Inno Setup, and so logic was added in commit
1651e55 to silently try to run
the old NSIS git-lfs-uninstaller.exe binary if it existed in
the same directory as git.exe, as found using the normal PATH
environment variables.

As all users of Git LFS should have upgraded to a recent version
installed with the Inno Setup installer, we can simply drop this
additional logic along with the GetExistingGitInstallation()
function of which it was the sole caller.

Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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

3 participants