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

git update-git-for-windows: optionally update from a fork #338

Merged
merged 1 commit into from
May 18, 2021

Conversation

dscho
Copy link
Member

@dscho dscho commented Apr 28, 2021

For the upcoming ARM64 support, as well as for the Microsoft fork of Git for Windows, it would be helpful if there was a way to update from the GitHub Releases of the respective fork instead of Git for Windows' own fork. This PR makes that easy.T o ask the updater to look for new versions at, say, https://github.com/dennisameling/git/releases, run the following command
as administrator:

git config \
   -f "$(git --exec-path)/git-update-git-for-windows.config" \
   update.fromFork dennisameling/git

Cc: @dennisameling

@dscho dscho requested a review from drizzd April 28, 2021 11:39
@dscho
Copy link
Member Author

dscho commented Apr 28, 2021

I should add that I did test this and it works as advertised.

@rimrul
Copy link
Member

rimrul commented Apr 28, 2021

This'll work for microsoft/git, but don't we need a way to specify the installer suffix for ARM64?

@dscho
Copy link
Member Author

dscho commented Apr 28, 2021

This'll work for microsoft/git, but don't we need a way to specify the installer suffix for ARM64?

Good point...

I think we will have to extend the .config support to allow specifying installer file names or some such. But I think we can safely do that on top of this PR, when @dennisameling has time (they indicated to be busy elsewhere until May 8th).

then
fork="$(git config -f "$0.config" update.fromFork)"
test -n "$fork" || {
echo "Could not find update.fromFork in $0.config" >&2
Copy link
Member

Choose a reason for hiding this comment

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

I get that end users shouldn't edit this config, but maybe we should fall back to the same behaviour we use when $0.config doesn't exist.

Wether the file exists or not, it's basically an unset config option.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. I now treat a .config file without update.fromFork the same as if the .config file was absent.

@dennisameling
Copy link
Contributor

But I think we can safely do that on top of this PR, when @dennisameling has time (they indicated to be busy elsewhere until May 8th).

I'm back! 😊 Created a PR on top of this one to detect ARM64: #340 - it simply checks if the /arm64/bin folder is present, just like we do in git-wrapper.c. I think that should be okay, but you might be aware of some edge-cases where this won't work.

Copy link
Contributor

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Can confirm that this works as described 👍🏼

For the upcoming ARM64 support, as well as for the Microsoft fork of Git
for Windows, it would be helpful if there was a way to update from the
GitHub Releases of the respective fork instead of Git for Windows' own
fork.

To facilitate that, let's introduce support for a
`git-update-git-for-windows.config` file next to the script: if it
exists, treat it as a Git config file and read the `update.fromFork`
value, and then continue with that value inserted into the URL
https://github.com/<fork>/releases.

Example: to ask the updater to look for new versions at
https://github.com/dennisameling/git/releases, run the following command
as administrator:

	git config \
	   -f "$(git --exec-path)/git-update-git-for-windows.config" \
	   update.fromFork dennisameling/git

This also overrides the label "Git for Windows" with "Git"; The
`update.gitLabel` setting in the same `.config` file can be used to
over-override this.

If the `.config` file does not contain any `update.fromFork`, the
file is ignored altogether.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented May 10, 2021

But I think we can safely do that on top of this PR, when @dennisameling has time (they indicated to be busy elsewhere until May 8th).

I'm back! 😊 Created a PR on top of this one to detect ARM64: #340 - it simply checks if the /arm64/bin folder is present, just like we do in git-wrapper.c. I think that should be okay, but you might be aware of some edge-cases where this won't work.

We do a little bit more in the Git wrapper: we also confirm that we're running on ARM64.

Therefore, it would be technically "more correct" if we introduced, say, --is-running-on-ARM64 in /cmd/git.exe and used that.

But I think that is overkill, so let's go with your solution for now.

Can confirm that this works as described 👍🏼

Could I ask you to test again? I changed the patch slightly in response to #338 (review).

@dscho dscho merged commit 23fc86c into git-for-windows:main May 18, 2021
@dscho dscho deleted the update-from-fork branch May 18, 2021 12:34
@dennisameling
Copy link
Contributor

@dscho Sorry for the late reply, was planning to test earlier, but was firefighting at work in the last few days 😅

Config:

[update]
	fromFork = dennisameling/git
	gitLabel = "Dennis's fork"

Output:

cmd/git.exe update-git-for-windows --gui
Dennis's fork 2.31.1.windows.1 (64bit)
Update 2.31.2.windows.1 is available

Toast also looking good:

image

Thanks! 🚀

@dscho
Copy link
Member Author

dscho commented May 18, 2021

No worries, I was quite distracted, too. Thanks for testing!

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.

3 participants