-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Create GCM 2.1.0 release #1235
Merged
Merged
Create GCM 2.1.0 release #1235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [lycheeverse/lychee-action](https://github.com/lycheeverse/lychee-action) from 1.5.4 to 1.6.1. - [Release notes](https://github.com/lycheeverse/lychee-action/releases) - [Commits](lycheeverse/lychee-action@4dcb8be...9ace499) --- updated-dependencies: - dependency-name: lycheeverse/lychee-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [lycheeverse/lychee-action](https://github.com/lycheeverse/lychee-action) from 1.5.4 to 1.6.1.
Because we use dotnet publish (which requires a specific framework) to create the payload of our .NET tool package, the package is tied to the framework specified. This means that it will only work with the corresponding .NET SDK for that framework. In light of this, update the .NET tool install instructions to specify that users must download the latest .NET 6.0 SDK as a prerequisite for installing the package, since that is the framework for which we currently publish. This will also need to be updated whenever we upgrade to a new Target Framework. Additionally, remove Windows as a supported .NET tool platform. Since we currently target .NET Framework rather than .NET, the package cannot actually be installed on Windows.
Because we use dotnet publish (which requires a specific framework) to create the payload of our .NET tool package, the package is tied to the framework specified. This means that it will only work with the corresponding .NET SDK for that framework. In light of this, update the .NET tool install instructions to specify that users must download the latest .NET 6.0 SDK as a prerequisite for installing the package, since that is the framework for which we currently publish. This will also need to be updated whenever we upgrade to a new Target Framework. Additionally, remove Windows as a supported .NET tool platform. Since we currently target .NET Framework rather than .NET, the package cannot actually be installed on Windows.
Ensure that we don't try to dispose of the pipe client in the `Trace2CollectorWriter` when it hasn't been created yet.
We should always try and drain standard output before waiting for the Git process to exit. We do this for all other calls that expect output, but missed the `TryGet` method.
Ensure that we don't try to dispose of the pipe client in the `Trace2CollectorWriter` when it hasn't been created yet.
We should always try and drain standard output before waiting for the Git process to exit. We do this for all other calls that expect output, but missed the `TryGet` method. This is important if Git ever writes out so much data to stdout that we buffer the output, and then Git will not exit until it can finish writing (and we'd be waiting for Git to exit).
Add the ability to include extra query parameters in the authorization code grant request. This allows consumers of the OAuth2Client to customise the login experience that may include vendor specific args.
If a username is specified in the remote URL then include this as a login hint when performing OAuth authentication for GitHub.
Add a unit test to ensure that we do not allow overriding or replacing standard OAuth2 query parameters.
GCM currently ignores ports in URIs. This means attempting to authenticate to a URI with a port can lead to some unexpected behavior (e.g. shortcutting the provider auto-detect process won't work, even if the provider is set in config). This change adds support for ports by updating GetGitConfigurationScopes() to recognize them. This change also updates GetRemoteUri() to recognize paths with queries and fragments, as that issue was uncovered during the implementation of the GetGitConfigurationScopes() fix. Without it, input paths containing queries and/or fragments get saved as part of Uri.Path, which converts the query '?' and the fragment '#' to url encoding.
GCM currently ignores ports in URIs. This means attempting to authenticate to a URI with a port can lead to some unexpected behavior (e.g. shortcutting the provider auto-detect process won't work, even if the provider is set in config). This change adds support for ports by updating GetGitConfigurationScopes() to recognize them. This change also updates GetRemoteUri() to recognize paths with queries and fragments, as that issue was uncovered during the implementation of the GetGitConfigurationScopes() fix. Without it, input paths containing queries and/or fragments get saved as part of Uri.Path, which converts the query '?' and the fragment '#' to url encoding. I validated these changes with unit tests for applicable scenarios and by running a locally-compiled version of GCM with tracing enabled and the following config set: `credential.http://localhost:7990/bitbucket.provider bitbucket` Trace logs showed that the override was recognized and auto-detection was skipped.
…1137) If a username is specified in the remote URL then include this as a login hint when performing OAuth authentication for GitHub. This can help users of multiple accounts select the correct account before the OAuth flow completes and returns a token for the wrong account. GitHub shows this nice prompt when a login hint is provided that does _not match the currently logged-in user_: ![image](https://user-images.githubusercontent.com/5658207/222230310-25c0e53e-ffab-4d42-bd4a-19d652e59835.png) At the same time, if there is no logged-in user, then the login page's username box is already filled in: ![image](https://user-images.githubusercontent.com/5658207/222231085-21c10d7d-bd38-4374-8ed8-da2e6f1429ee.png) The login hint is specified to GitHub via the extra `login` query parameter to the authorization endpoint.
The GitCredentialManager org is being re-named to git-ecosystem. In light of this, update references to GitCredentialManager to instead reference git-ecosystem.
Bumps [mjcheetham/update-homebrew](https://github.com/mjcheetham/update-homebrew) from 1.2 to 1.3. - [Release notes](https://github.com/mjcheetham/update-homebrew/releases) - [Commits](mjcheetham/update-homebrew@v1.2...v1.3) --- updated-dependencies: - dependency-name: mjcheetham/update-homebrew dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [mjcheetham/update-homebrew](https://github.com/mjcheetham/update-homebrew) from 1.2 to 1.3.
The GitCredentialManager org is being re-named to git-ecosystem. In light of this, update references to GitCredentialManager to instead reference git-ecosystem.
Create static class to manage session ids (sids). This allows us to immediately set up GCM's sid in Program.cs to ensure Git processes that are executed before Trace2 tracing is set up have the correct parent sid. We also use it to give helper exes unique child sids. Although this will also execute when TRACE2 tracing is not enabled, it should not be an issue - it will simply set the (unused) GIT_TRACE2_PARENT_SID varible for the process to a GUID.
Our current strategy of using a Stream to write to files for Trace2 tracing is not working - Git processes are overwriting GCM's data and vice versa. To resolve this, switch from using the Trace2StreamWriter to write to files to a dedicated Trace2FileWriter class. This also includes a refactor to add a parent base class for Trace2Writers for shared functaionality between the Trace2StreamWriter and Trace2FileWriter.
The TRACE2 convention is for multi-word event names to be written in snake case (all lower case with words separated by underscores). While up until this point we've had single-word event names, this series will add the multi-word child start and child exit events. In anticipation of that, this change adds a string extension that will be used in later patches to write multi-word enum values in snake case.
When a new process is created, standard error can optionally be redirected. This is useful in situations where the output is verbose and should be suppressed rather than output to the console, for example. However, this can cause an issue with TRACE2 deadlocks, as Git writes to standard error and GCM does nothing. This change corrects this by not redirecting standard error for Git-related processes (which also extends to places such as Environment.cs, given Git's reliance on envars). Finally, there are some places where we should still redirect standard error for the reason mentioned above. For these scenarios, comments have been added to help readers understand the purposeful nature of these redirects.
Diagnostics classes are dependent on multiple pieces of the CommandContext. Adding child start and child exit events will mean yet another dependency (Trace2). To simplify in anticipation of this, update Diagnostic classes to take in the CommandContext and access applicable dependencies from it.
The TRACE2 child start and child exit events will require some refactoring around the way GCM handles processes to provide finer-grained control and the ability to capture process-related information. In preparation for this, we need to do some re-organization of the Trace2 code. With this change we: 1. Move TRACE2 tracing startup into Program.cs for GCM and the helper exes. This allows us to start tracing earlier and capture more events. 2. Add an Initialize() method to Trace2.cs to detect whether Trace2 tracing is enabled and set long-lived variables. This also sets a new _initialized property that allows us to determine whether Trace2 tracing has been initialized. Trace2 tracing is disabled until _initialized is set to true by the Initialize() method. 3. Change the name of TryParseSettings() to InitializeWriters() for improved clarity. 4. Pass the CommandContext to Trace2 to use its Streams and Environment.
Wrap the Process class to allow finer-grained control for writing future process-related TRACE2 events. Pull process-relevant information out of IEnvironment/Environment and into its own process-oriented ProcessManager class, interface, and child classes. This allows us to maintain a functional, non-circular order of dependencies with our process wrapper. Add a ChildProcess class to wrap certain functionality of Process.cs. This gives us the level of control required to write child process information to TRACE2 in the final patch in this series. Update all current invocations of Process to instead use this wrapper.
Write the TRACE2 child_start and child_exit events for each child process invoked by GCM.
This change adds the `child_start` and `child_exit` events to GCM's `TRACE2` tracing system. The first patches are either bug fixes or prep for the bigger refactors involved in making child start and exit work. The first patch puts sid-related logic into its own class, which allows us to immediately set up GCM's sid in `Program.cs` to ensure Git processes that are executed before `TRACE2` tracing is set up have the correct parent sid. The next patch adds a `FileWriter` class and uses it (rather than `Trace2StreamWriter`) to write to files, which ensures processes aren't overwriting one another's data. The third patch adds a snake casing string extension to ensure we write multi-word enum values according to the `TRACE2` convention. The fourth patch ensures we _do not_ redirect stderr for Git-related processes, as it was discovered to cause deadlocks when `TRACE2` was enabled. The fifth patch refactors GCM's diagnostics to depend on an instance of `CommandContext` rather than individual dependencies within `CommandContext`. The sixth patch re-configures `TRACE2` to depend on an instance of `CommandContext` rather than individual dependencies within `CommandContext`, to start up earlier, and for operations to no-op if `Trace2` has not yet been initialized (i.e. when child `git config` processes run before we're able to set up tracing). The seventh patch wraps `Process.cs` to give us the needed level of control to write `TRACE2` child process start and exit events. The eighth patch (finally) adds the new child start and exit events.
We are creating a `WindowsProcessManager` implementation on macOS and a non-Windows/WSL aware, plain `ProcessManager` on Windows - this is the reverse of what we should do. Fix this.
We are creating a `WindowsProcessManager` implementation on macOS and a non-Windows/WSL aware, plain `ProcessManager` on Windows - this is the reverse of what we should do. Fix this.
To keep the size of the Windows distribution small, directly target the Win32 Avalonia packages rather than the Desktop package that includes macOS and Linux dependencies.
) Today GCM has an architecture that delegates all GUI prompting to (bundled) external executables, using WPF-based helpers on Windows and Avalonia UI helpers on Mac and Linux. Each host provider has its own GUI helper executable, meaning we currently bundle four (GitHub.UI, GitLab.UI, Atlassian.Bitbucket.UI, git-credential-manager.ui [for generic auth]) helpers in addition to the core git-credential-manager CLI executable. On Linux we use the "publish as a single-file" model for each of these executables, meaning we have a copy of the CLR included for each executable! That means five CLRs! Now that the Avalonia UI-based GUI helpers have matured, we can look at dropping the WPF helpers and reduce the effort required to ship new GUI by 50% (we no longer need to do everything twice: once with WPF and once with Avalonia UI). In addition, in a recent spike, I found that it may be possible to bring the Avalonia UI helpers in-process, with nearly zero net-new code. This would help reduce our installation footprint on Linux down to approximately 1/4 of the current size. The single executable model would also help decrease build times, and increase end-user performance since we no longer pay the process startup costs using external processes! We need to be mindful that this change may result in a small increase in the Windows installation as we now bring in extra libraries used to support Avalonia UI, whereas previously we relied on WPF libraries as included with Windows 'for free'. This is expected to be minimal. As for the current GUI extensibility model, we are not aware of any consumers, but it would not be much effort to keep the existing external process model, but now just default to using our in-process GUI options in the absence of external configuration. This still gives tools like VS or VSCode options to integrate native UI for GCM in the future. Initially, we are still keeping the WPF helpers around on Windows but disabled by default. We can drop these after at least one release using the in-proc Avalonia helpers. To switch back to the WPF helpers, use `credential.devUseLegacyUIHelpers` or `GCM_DEV_USELEGACYUIHELPERS`.
Add the ability to configure MSAL to use the default OS account when the broker is enabled. Default to disabled.
Detect when we are in a Microsoft Dev Box environment, and if we are, then default to enabling the default OS account setting and enabling WAM.
Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/DavidAnson/markdownlint-cli2-action) from 9.0.0 to 10.0.0. - [Release notes](https://github.com/DavidAnson/markdownlint-cli2-action/releases) - [Commits](DavidAnson/markdownlint-cli2-action@5b7c9f7...cdfad95) --- updated-dependencies: - dependency-name: DavidAnson/markdownlint-cli2-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Add options for enabling tracing and debugging in Git configuration. Today the below options were only supported via environment variable: - GCM_TRACE - GCM_TRACE_SECRETS - GCM_TRACE_MSAUTH - GCM_DEBUG
Add options for enabling tracing and debugging in Git configuration. Today the below options were only supported via environment variable: - `GCM_TRACE` - `GCM_TRACE_SECRETS` - `GCM_TRACE_MSAUTH` - `GCM_DEBUG`
Add Trace2 config/env var options to GCM's configuration/environment documentation to improve discoverability.
Add Trace2 config/env var options to GCM's configuration/environment documentation to improve discoverability.
…0.0.0 (#1223) Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/DavidAnson/markdownlint-cli2-action) from 9.0.0 to 10.0.0.
Remove Nerdbank.GitVersioning dependency from project in favor of a new VERSION file, which will be updated ahead of each release. Versioning in this file will begin with the version we plan to use for the next release: 2.1.0.0. This change involves the addition of a new custom MSBuild task, which reads in the contents of the VERSION file, converts it to a Version object, and then sets the various version-related MSBuild properties with the correct value (some with the `Revision` component appended, others without). Note that there is a bug in MSAL [1] that causes build failures for projects without dependencies with this change. We add Newtonsoft.Json as a global dependency in Directory.Build.props to work around this problem until the fix is released. [1]: AzureAD/microsoft-authentication-library-for-dotnet#4108
Update release workflow to use VERSION file for versioning, rather than Nerdbank.GitVersioning.
Remove Nerdbank.GitVersioning tasks (which have been superceded by the use of the new VERSION file) from the release workflow.
Remove fetch-depth from checkout actions, since that was required by Nerdbank.GitVersioning. Also take this opportunity to use consistent version and naming for checkout actions.
With the [plan to migrate GCM to a formula for release via `Homebrew/homebrew-core`](#1102), it became clear that [Nerdbank.GitVersioning](https://github.com/dotnet/Nerdbank.GitVersioning) was no longer an option for versioning the project. This is because `Nerdbank.GitVersioning` requires history to calculate things like ['Git height'](https://github.com/dotnet/Nerdbank.GitVersioning#what-is-git-height), and the formula requires a tarball to build, which, of course, has no history. This change pivots GCM to the simpler strategy of using a version specified in a `VERSION` file at root. This version will be updated by maintainers prior to release - giving them more granular control of versioning, which in turn allows for versioning to be more predictable (i.e. maintainers will know what the version will be before publication of the release). The version specified in the file is the one slated for the next release: `2.1.0.0`. **Note:** This change [fails on Windows](https://github.com/ldennington/git-credential-manager/actions/runs/4824686907) due to a [bug in MSAL](AzureAD/microsoft-authentication-library-for-dotnet#4108) unless we ensure all projects have at least 1 dependency. We are working around this issue by adding Newtonsoft.Json as this dependency (since we were already shipping it anyway).
Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/DavidAnson/markdownlint-cli2-action) from 10.0.0 to 10.0.1. - [Release notes](https://github.com/DavidAnson/markdownlint-cli2-action/releases) - [Commits](DavidAnson/markdownlint-cli2-action@cdfad95...bb4bb94) --- updated-dependencies: - dependency-name: DavidAnson/markdownlint-cli2-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…10.0.1 (#1232) Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/DavidAnson/markdownlint-cli2-action) from 10.0.0 to 10.0.1.
Add the ability to configure MSAL to use the default OS account when the broker is enabled. Also detect when we are in a Microsoft Dev Box environment, and if we are, then default to enabling the new setting (and enable WAM). Show a confirmation prompt before continuing to use the current OS account, which is similar to how Microsoft Teams operates. <img width="888" alt="windows-defaultaccount" src="https://user-images.githubusercontent.com/5658207/234346956-b08eb43f-c964-4978-84ec-a0f75f021f08.png"> Left: Avalonia UI, Right: fallback WPF window Fixes #917
ldennington
approved these changes
May 2, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - thanks for the excellent notes!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
WindowsProcessManager
on Windows #1146)ProcessManager
#1177)