Rename entry executable to drop "-core" suffix #551

mjcheetham · 2021-11-23T13:17:45Z

UPDATE

Rename the main entry executable from git-credential-manager-core(.exe) to simply git-credential-manager(.exe), now that the older GCM4W has been removed from the Git for Windows project as an option (and the GCMC project has been renamed).

We do not rename the Debian package ID, nor the Homebrew Cask name yet as this will require more thought forr migration.

To help with migration somewhat, create symlinks and shim/copy-executables for the original executable name "git-credential-manager-core(.exe)" for consumers who have not updated to the new version.

We detect if the consumer is launching us via the "-core" symlink or executable shim by consulting the platform-native APIs to get the original "argv[0]". All of the .NET APIs sadly don't give us the real "argv[0]", so we need to use native APIs...

If we detect use of the old name, print a warning that the user should update their configuration, and a help link for more information.

Link to rendered "rename" doc: https://github.com/mjcheetham/git-credential-manager/blob/exec-rename/docs/rename.md

.github/workflows/release-homebrew.yaml

vtbassmatt · 2021-11-23T13:24:48Z

Is there a deprecation plan for the shims? It seems like a lot of hassle for low rewards if we have to carry those indefinitely. But if we can document their disappearance in, say, a year...

ldennington · 2021-11-23T14:08:03Z

Do we have a way of knowing when VS takes a new mingit version?

ldennington

Exciting times! 🚀

.azure-pipelines/templates/osx/pack.signed/step5-dist.yml

src/linux/Packaging.Linux/shim.sh

src/windows/Installer.Windows/Setup.iss

mjcheetham · 2021-11-23T14:49:45Z

Is there a deprecation plan for the shims? It seems like a lot of hassle for low rewards if we have to carry those indefinitely. But if we can document their disappearance in, say, a year...

That's a good point. I know @dscho was thinking of removing the Windows shim script from Git for Windows "at some future date".

Would you put the "this is temporary" message in the warnings too?

Regarding the rewards; I think this is really most important for Git for Windows users, and those who have installed outside of Homebrew (tarballs, macOS pkg, or Debian deb). Homebrew updates will do an uninstall first of the old one, so keeping clean.

mjcheetham · 2021-11-23T14:50:44Z

Do we have a way of knowing when VS takes a new mingit version?

Major releases certainly, and sometimes minor releases. It requires a full test pass on their side of things and is an expensive thing to do. They usually also update when we advise them to, for security reasons for example.

Rename the main entry executable from git-credential-manager-core(.exe) to simply git-credential-manager(.exe), now that the older GCM4W has been removed from the Git for Windows project as an option (and the GCMC project has been renamed). We do not rename the Debian package ID, nor the Homebrew Cask name yet as this will require more thought for migration.

ldennington

OMG IT'S HAPPENING! 🍾

ldennington · 2022-10-14T15:04:33Z

src/shared/Core/ApplicationBase.cs

+            string argv0 = PlatformUtils.GetNativeArgv0()
+                           ?? Process.GetCurrentProcess().MainModule?.FileName
+                           ?? Environment.GetCommandLineArgs()[0];
+
+            if (Path.IsPathRooted(argv0))
            {
-                return Path.ChangeExtension(candidatePath, null);
+                return argv0;
            }

-            return candidatePath;
-#endif
+            return Path.GetFullPath(
+                Path.Combine(Environment.CurrentDirectory, argv0)
+            );


Nice simplification!

It's changes like these (avoiding .NET and calling platform native APIs) that makes me wonder where the balance of "help" vs "hinder" .NET is for us... 🤔

ldennington · 2022-10-14T15:47:46Z

src/shared/Core/Interop/Windows/Native/Shell32.cs

+        /// <summary>
+        /// Parses a Unicode command line string and returns an array of pointers
+        /// to the command line arguments, along with a count of such arguments,
+        /// in a way that is similar to the standard C run-time argv and argc values.
+        /// </summary>
+        /// <param name="lpCmdLine">
+        /// Pointer to a null-terminated Unicode string that contains the full command line.
+        /// If this parameter is an empty string the function returns the path to the current executable file.
+        /// </param>
+        /// <param name="pNumArgs">
+        /// Pointer to an int that receives the number of array elements returned, similar to argc.
+        /// </param>
+        /// <returns>A pointer to an array of LPWSTR values, similar to argv.</returns>


These comments are really helpful, thanks!

They came directly from the Windows API docs online 😅

Even better 😁

ldennington · 2022-10-14T15:51:47Z

src/shared/Core/PlatformUtils.cs

+            string cmdline = File.ReadAllText("/proc/self/cmdline");
+            return cmdline.Split('\0')[0];


Wow this is just so easy on Linux lol.

src/shared/Git-Credential-Manager/Program.cs

mjcheetham · 2022-10-14T16:46:23Z

docs/rename.md

+![Git Credential Manager Core renamed](img/gcmcore-rename.png)
+
+At the time, the actual exectuable name was not updated and continued to be
+`git-credential-manager-core`. As of [VERSION][rename-ver], the executable has


The VERSION string is a placeholder for when we make the first release and know the version number of GCM.

I've added a reminder alongside my reminder for the next release (all the reminders!) to update this along with the WINGIT_VERSION variable.

mjcheetham · 2022-10-14T16:46:49Z

docs/rename.md

+### Git for Windows
+
+If you are using GCM bundled with Git for Windows (recommended), you should make
+sure you have updated to at least version WINGIT_VERSION.


..similarly, the WINGET_VERISON will be updated once we know the version number of the Git for Windows release that ships GCM with the new name.

mjcheetham · 2022-10-14T16:47:50Z

docs/rename.md

+These links will remain until _two_ major Git versions are released after GCM
+VERSION, _**at which point the symlinks will no longer be included**_.


I think giving two major Git versions is a good runway. Thoughts? @ldennington @dscho

(By "major" of course I mean y in x.y.z.. not x!)

Good clarification! I am fine with this timeline.

mjcheetham · 2022-10-14T22:01:42Z

I'm going to wait until @dscho is back for their opinion on this solution before merging. We want to make sure this will be OK with Git for Windows.

dscho

Yes!!!

dscho · 2022-10-18T12:16:19Z

src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs

+            Assert.Single(actualValues);
+            Assert.Equal("true", actualValues[0]);
+        }
+


I'm not quite sure whether this test was added intentionally? I might be misinterpreting, but nothing in that test speaks "drop the -core suffix" to me... Likewise with AzureReposHostProvider_UnconfigureAsync_User_Windows_UseHttpPathSetAndManagerHelper_RemovesEntry below.

These tests are to ensure that, on Windows, when uninstalling GCM (as installed from a standalone installer), we do NOT delete the useHttpPath setting that was placed in the --system config file by Git for Windows.

The new tests check to see if, when helper = manager, we don't remove the useHttpPath setting.

The existing tests check we don't remove the useHttpPath setting when helper = manager-core. This would be the case when installing and then uninstalling GCM standalone with an older version of Git for Windows present.

src/linux/Packaging.Linux/build.sh

src/shared/Core/Interop/Windows/Native/Shell32.cs

Create symlinks and shim/copy-executables for the original exec "git-credential-manager-core(.exe)" name for consumers who have not updated to the new name. We detect if the consumer is launching us via the "-core" symlink or executable shim by consulting the platform-native APIs to get the original "argv[0]". All of the .NET APIs sadly don't give us the real "argv[0]", so we need to use native APIs... If we detect use of the old name, print a warning that the user should update their configuration, and a help link for more information.

Add documentation with information about the GCM.exe rename, and how to fix-up any configuration after the change. The aka.ms/gcm/rename shortlink points to this document.

Changes: - Check for broken links in documentation (#700) - Support macOS `arm64` installs via Homebrew (#798) - Validate installers before publishing (#813) - Auto-generate maintainer away notification issues (#842) - Install dotnet via Jammy feeds on Ubuntu 22.04 and greater (#839) - Access Azure storage account using service principle credentials (#851) - Update documentation to use reference-style links (#680) - Unify documentation line length (#862) - Add generic username/password UI (#871) - Bitbucket DC OAuth support (#607) - Distribute GCM as a dotnet tool (#886) - Drop `-core` suffix from entry executable #551 - Speed up build graph (#924)

…ion (#951) When we introduced the rename warning in #551, there was broken logic for resolving the actual invoked program name on Mac and Linux. **Windows** We continue to use `CommandLineToArgvW(GetCommandLine(), ..)` to return the _absolute, full path_ to the entry executable. **macOS** Switch from `_NSGetArgv()` to `_NSGetExecutablePath(..)` which, like the Windows APIs above, returns the full path to the entry executable (or symlink). **Linux** As there is no equivalent to `CommandLineToArgvW` or `_NSGetExecutablePath` here, we instead do some path computation based on the form of `argv[0]` that we get from `/proc/self/cmdline`. - If the value is an absolute path, just use that. - If the value is relative to the current directory (`./name`) then combine this with the current directory. - If the value contains a directory separator (`dir/name`) then also resolve this from the current directory. - Otherwise, this `argv[0]` value must have been a file name (`name`) resolved from the `$PATH`. If we still don't manage to resolve from the `$PATH`, try and resolve the symlink `/proc/self/exe` that points to the executable image that was loaded from disk. Note that we may miss any intermediate link names here, but it's better than nothing. --- In addition, we also never tested the .NET Tool scenario after updating the `ICommandContext.ApplicationPath` to use the entry executable name, whereas previously this was computed from the .NET assembly path. Introduce a separate concept, the `InstallationDirectory` that always points to the home of the core assemblies. This is used for resolving things like in-box UI helpers.

Fixed path for user-only installation of GCM for Windows. Seems #551 forgot to update this path. version: v2.1.2 imagePath: `C:\Users\user` ``` $ git config --global credential.helper "/mnt/c/Users/user/AppData/Local/Programs/Git\ Credential\ Manager/git-credential-manager.exe" $ git pull Already up to date. ```

mjcheetham requested review from dscho, derrickstolee, vtbassmatt and ldennington November 23, 2021 13:17

dscho reviewed Nov 23, 2021

View reviewed changes

.github/workflows/release-homebrew.yaml Outdated Show resolved Hide resolved

vtbassmatt approved these changes Nov 23, 2021

View reviewed changes

ldennington approved these changes Nov 23, 2021

View reviewed changes

.azure-pipelines/templates/osx/pack.signed/step5-dist.yml Outdated Show resolved Hide resolved

src/linux/Packaging.Linux/shim.sh Outdated Show resolved Hide resolved

src/windows/Installer.Windows/Setup.iss Show resolved Hide resolved

mjcheetham force-pushed the exec-rename branch from 4d62ad6 to b530e70 Compare November 23, 2021 15:39

Joejoejoejo mentioned this pull request Dec 14, 2021

> Is there a deprecation plan for the shims? It seems like a lot of hassle for low rewards if we have to carry those indefinitely. But if we can document their disappearance in, say, a year... #571

Closed

wolf99 mentioned this pull request Apr 15, 2022

Lint markdown #668

Merged

mjcheetham force-pushed the exec-rename branch from b530e70 to a6923c5 Compare May 24, 2022 11:33

ldennington mentioned this pull request May 25, 2022

Migrate release processes to GitHub Actions #707

Merged

mjcheetham self-assigned this Jun 1, 2022

ivroyalblue approved these changes Jul 7, 2022

View reviewed changes

hickford mentioned this pull request Oct 13, 2022

dotnet tool: rename package #901

Merged

mjcheetham force-pushed the exec-rename branch from a6923c5 to f4ef4af Compare October 14, 2022 00:12

mjcheetham force-pushed the exec-rename branch from f4ef4af to c5b0164 Compare October 14, 2022 02:44

mjcheetham requested review from ldennington, dscho and vtbassmatt October 14, 2022 02:44

ldennington approved these changes Oct 14, 2022

View reviewed changes

mjcheetham force-pushed the exec-rename branch 3 times, most recently from 0e6761a to 644336b Compare October 14, 2022 16:40

mjcheetham commented Oct 14, 2022

View reviewed changes

dscho approved these changes Oct 18, 2022

View reviewed changes

mjcheetham added 2 commits October 24, 2022 11:53

docs: add documentation about the GCM.exe rename

a47e1ce

Add documentation with information about the GCM.exe rename, and how to fix-up any configuration after the change. The aka.ms/gcm/rename shortlink points to this document.

mjcheetham force-pushed the exec-rename branch from 644336b to a47e1ce Compare October 24, 2022 18:53

mjcheetham merged commit 02204ac into git-ecosystem:main Oct 24, 2022

mjcheetham deleted the exec-rename branch October 24, 2022 19:23

ldennington mentioned this pull request Oct 27, 2022

WSL documentation updated #926

Closed

ldennington mentioned this pull request Nov 3, 2022

Release 2.0.866 #944

Merged

mjcheetham mentioned this pull request Nov 7, 2022

Fix problems with application path and installation directory resolution #951

Merged

ldennington mentioned this pull request Nov 8, 2022

Fix command-line instructions for .deb install #940

Closed

This comment was marked as spam.

Sign in to view

yuki-js mentioned this pull request Jul 5, 2023

docs: fix path of user-only installation #1317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename entry executable to drop "-core" suffix #551

Rename entry executable to drop "-core" suffix #551

mjcheetham commented Nov 23, 2021 •

edited

Loading

vtbassmatt commented Nov 23, 2021

ldennington commented Nov 23, 2021

ldennington left a comment

mjcheetham commented Nov 23, 2021

mjcheetham commented Nov 23, 2021

ldennington left a comment

ldennington Oct 14, 2022

mjcheetham Oct 14, 2022

ldennington Oct 14, 2022

mjcheetham Oct 14, 2022

ldennington Oct 14, 2022

ldennington Oct 14, 2022

mjcheetham Oct 14, 2022

ldennington Oct 14, 2022

mjcheetham Oct 14, 2022

mjcheetham Oct 14, 2022 •

edited

Loading

ldennington Oct 14, 2022

dscho Oct 18, 2022

mjcheetham commented Oct 14, 2022

dscho left a comment

dscho Oct 18, 2022

mjcheetham Oct 24, 2022 •

edited

Loading

This comment was marked as spam.

This comment was marked as spam.

		string cmdline = File.ReadAllText("/proc/self/cmdline");
		return cmdline.Split('\0')[0];

		These links will remain until _two_ major Git versions are released after GCM
		VERSION, _at which point the symlinks will no longer be included_.

Rename entry executable to drop "-core" suffix #551

Rename entry executable to drop "-core" suffix #551

Conversation

mjcheetham commented Nov 23, 2021 • edited Loading

vtbassmatt commented Nov 23, 2021

ldennington commented Nov 23, 2021

ldennington left a comment

Choose a reason for hiding this comment

mjcheetham commented Nov 23, 2021

mjcheetham commented Nov 23, 2021

ldennington left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcheetham Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcheetham commented Oct 14, 2022

dscho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcheetham Oct 24, 2022 • edited Loading

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

mjcheetham commented Nov 23, 2021 •

edited

Loading

mjcheetham Oct 14, 2022 •

edited

Loading

mjcheetham Oct 24, 2022 •

edited

Loading