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

Upgrade git #10022

Closed
rafeca opened this issue Jun 16, 2020 · 9 comments · Fixed by #10077
Closed

Upgrade git #10022

rafeca opened this issue Jun 16, 2020 · 9 comments · Fixed by #10077
Assignees

Comments

@rafeca
Copy link
Contributor

rafeca commented Jun 16, 2020

This issue tracks the investigation to upgrade the version of git that GitHub Desktop is currently using.

Current git version

As of now (GitHub Desktop v2.5.2), we're using dugite v1.88.6 which includes git v2.23.3 on macOS and v2.23.0.windows.5 on Windows.

Candidates to upgrade

I've considered 2 different versions to use for the upgrade:

  1. v2.27.0 (released on June 1st 2020).
  2. v2.26.2 (released on April 20th 2020, while v2.26.0 was released on March 23rd 2020).

Interesting factors to consider

  • We've historically avoided upgrading to the latest git version to avoid potential instabilities.
  • As mentioned by @dscho in Slack, "Git for Windows v2.27.0 seems to be not quite as robust as the median Git for Windows release". Given that there's currently no other v2.27.x version of Git for Windows, it seems fair to avoid v2.27.0.
  • v2.25.0 contains a fix for Cloning: error: filename in tree entry contains backslash" '\' #9904.
  • v2.27.0 contains a fix for GitHub Desktop unable to fetch ("url has no scheme") #9597 on macOS.
  • v2.26.0 enabled by default the transport protocol version 2. Given that this got reverted on v2.27.0 due to "some remaining rough edges" (more info), we should consider changing the default if we upgrade to v2.26.2

Recommendation

My recommendation is to upgrade to git v2.26.2

Regarding the default protocol version, after talking to @niik we found a couple of ways to override its default value to be v1 (the same value as older or newer git versions):

  1. Override the system gitconfig to set the appropriate default protocol version. This can be easily done for Windows (we already do it for other config settings but it would need some patching of the git sourcecode for macOS (basically applying this commit), since there doesn't seem to be a system-wide gitconfig available.
  2. Override the config parameter from Desktop via a CLI argument (like we're doing in the opposite way right now for GitHub repositories). This would be a much simpler change.

I personally think that it should be fine to not override the default protocol version because:

  • In Desktop we've been already setting it to v2 for GitHub repositories for a long time without getting issues reported
  • The "rough edges" doesn't seem to be general problematic issues. Based on this commit message the revert was done "to be cautious", and the fact that the revert has not been backported to v2.26.x also signals that the potential issues with protocol version 2 are not that severe.

In case we still want to be cautious and override the parameter, I suggest doing it from Desktop to reduce the number of custom changes in dugite-native.


Summarized changelog since v2.23.3

This is the raw list of changelog items that I considered somewhat relevant between our current git version and the latest (v2.27.0) git version.

v2.24.0

Backward compatibility changes:

  • "filter-branch" is showing its age and alternatives are available. From this release, we started to discourage its use and hint people about filter-repo.

Interesting new features:

  • "git fetch" learned "--set-upstream" option to help those who first clone from their private fork they intend to push to, add the true upstream via "git remote add" and then "git fetch" from it.

Interesting bug fixes:

v2.25.0

Interesting bug fixes:

  • An earlier update to Git for Windows declared that a tree object is invalid if it has a path component with backslash in it, which was overly strict, which has been corrected. The only protection the Windows users need is to prevent such path (or any path that their filesystem cannot check out) from entering the index.

v2.26.0

Backward compatibility changes:

  • "git rebase" uses a different backend that is based on the 'merge' machinery by default. There are a few known differences in the behaviour from the traditional machinery based on patch+apply.

    If your workflow is negatively affected by this change, please report it to [email protected] so that we can take a look into it. After doing so, you can set the 'rebase.backend' configuration variable to 'apply', in order to use the old default behaviour in the meantime.

  • The transport protocol version 2 becomes the default one.

Interesting bug fixes:

  • "git fetch" over HTTP walker protocol did not show any progress output. We inherently do not know how much work remains, but still we can show something not to bore users.

v2.27.0

Backward compatibility notes:

  • The transport protocol version 2, which was promoted to the default in Git 2.26 release, turned out to have some remaining rough edges, so it has been demoted from the default.

  • When "git describe C" finds that commit C is pointed by a signed or annotated tag, which records T as its tagname in the object, the command gives T as its answer. Even if the user renames or moves such a tag from its natural location in the "refs/tags/" hierarchy, "git describe C" would still give T as the answer, but in such a case "git show T^0" would no longer work as expected. There may be nothing at "refs/tags/T" or even worse there may be a different tag instead.

    Starting from this version, "git describe" will always use the "long" version, as if the "--long" option were given, when giving its output based on such a misplaced tag to work around the problem.

  • "git pull" issues a warning message until the pull.rebase configuration variable is explicitly given, which some existing users may find annoying---those who prefer not to rebase need to set the variable to false to squelch the warning.

Interesting new features:

  • A handful of options to configure SSL when talking to proxies have been added.

  • "git merge" learns the "--autostash" option.

Interesting bug fixes:

  • Fix "git checkout --recurse-submodules" of a nested submodule hierarchy.

  • "git pull --rebase" tried to run a rebase even after noticing that the pull results in a fast-forward and no rebase is needed nor sensible, for the past few years due to a mistake nobody noticed.

  • Recent updates broke parsing of "credential.." where is not a full URL (e.g. [credential "https://"] helper = ...) stopped working, which has been corrected.

    With the recent tightening of the code that is used to parse various parts of a URL for use in the credential subsystem, a hand-edited credential-store file causes the credential helper to die, which is a bit too harsh to the users. Demote the error behaviour to just ignore and keep using well-formed lines instead.

@dscho
Copy link
Contributor

dscho commented Jun 16, 2020

* "git pull" issues a warning message until the pull.rebase configuration variable is explicitly given, which some existing users may find annoying---those who prefer not to rebase need to set the variable to false to squelch the warning.

I meant to look into releasing Git for Windows v2.27.0(2) anyway (what with git svn and the symbolic link issues fixed), and we could use this opportunity to set pull.rebase=false in MinGit's system gitconfig. That would at least address the warning...

What do you think, @rafeca?

@rafeca
Copy link
Contributor Author

rafeca commented Jun 17, 2020

I meant to look into releasing Git for Windows v2.27.0(2) anyway (what with git svn and the symbolic link issues fixed), and we could use this opportunity to set pull.rebase=false in MinGit's system gitconfig. That would at least address the warning...

What do you think, @rafeca?

That sounds good to me, I don't think that having pull.rebase=false will affect much Desktop since we won't show these warnings but may be a good thing for Git for Windows users. Do you know the context behind requiring the user to explicitly set this config variable in git core?

@rafeca
Copy link
Contributor Author

rafeca commented Jun 17, 2020

To be clear, I don't think we should upgrade Desktop directly to v2.27.0 at this moment even if these two things get fixed, since we try to avoid upgrading to the latest git version (unless @outofambit or @niik think otherwise).

@dscho
Copy link
Contributor

dscho commented Jun 18, 2020

Do you know the context behind requiring the user to explicitly set this config variable in git core?

There were a couple voices that thought that a rebase flow was much better than a merge flow, others disagreed vehemently, and the compromise found on the list was this: show a warning to everybody and force them to choose ;-)

To be clear, I don't think we should upgrade Desktop directly to v2.27.0 at this moment even if these two things get fixed, since we try to avoid upgrading to the latest git version (unless @outofambit or @niik think otherwise).

That's actually good because I would like to let things simmer a little longer before releasing v2.27.0(2), to increase the chances for a solid user experience (at least more solid than v2.27.0).

@Jccp1983

This comment has been minimized.

@Jccp1983

This comment has been minimized.

@Jccp1983

This comment has been minimized.

@desktop desktop temporarily blocked Jccp1983 Jun 26, 2020
@MadMid39

This comment has been minimized.

@rafeca rafeca self-assigned this Jul 14, 2020
@ZEARMENI

This comment was marked as spam.

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 a pull request may close this issue.

5 participants