-
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
release: package gcm as dotnet tool #886
Conversation
4d00d59
to
a3a6896
Compare
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.
This looks like a nice and clean result. I'm sure the journey to get here wasn't easy, but thanks for putting in that effort.
<!-- A "dummy" version is required, even though we override in csproj --> | ||
<version>0.0.0</version> | ||
<description>Secure, cross-platform Git credential storage with authentication to Azure Repos, GitHub, and other popular Git hosting services.</description> | ||
<authors>The Git Fundamentals Team</authors> |
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.
Should we point to the gitcredentialmanager
org instead of this team name?
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.
I'm not sure. This is unfortunately a required field, or else I would have removed it. Here's what the docs recommend:
authors
A comma-separated list of packages authors, matching the profile names on nuget.org. These are displayed in the NuGet Gallery on nuget.org and are used to cross-reference packages by the same authors.
Let me take a look at some other packages and think about what we want to do about this.
Exciting! Is there a .nupkg file to try out? |
The package itself is cross platform, right? |
001c80e
to
7291505
Compare
The best way to dogfood at the moment is to build a dummy version of the package and install it locally (it's only published to my fork's GitHub Packages registry at the moment which has some...issues). For the local install, from the root of your gcm repo run:
I will keep you posted with new instructions if we're able to resolve the GitHub registry issues. Edit: Issues with my fork's registry are resolved! See the updated PR description for instructions to install. |
Going to go ahead and promote this to a full PR - I feel good about our process for dogfooding from the registry while we wait to publish to NuGet. |
This is not a cross platform package. It contains compiled/self-contained binaries specifically for Linux (the To make this a cross platform package we'd need to However, the goal here with this change is to help Linux users specifically. For Windows and macOS we already have robust installation methods (via Git for Windows and Homebrew respectively), so the cross platform idea would be a future idea that we'd welcome community contribution for. |
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.
This looks and works great (tested on one of my WSL instances)!
I had a few comments and questions around package metadata and which scripts are called on a dev box.
Installed successfully. Tested git push to GitHub. Package size is 147 MB (too big to upload here). Install size is 493 MB. Diagnostic "Microsoft authentication (AAD/MSA)" failed:
|
For me, that's prohibitively big. I want to use GCM everywhere I use Git including where disk space is precious. Git is 35 MB. The .NET runtime is 70 MB. Have you considered omitting the UI helpers? Advantages:
|
@mjcheetham and I talked this over. The best path forward is probably to go ahead and make this cross-plat and update the way we call the helper exes. Hopefully no longer bundling the runtime multiple times will bring this down to an acceptable size. Planning to get as far as I can on this today. @hickford: A new package has been pushed to my registry ( |
27f8911
to
94439b7
Compare
@mjcheetham @hickford - I believe this is now in a state where it's prepared for final review. |
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.
Overall this looks and works great from my testing! 🎉
I've left some comments about how we locate and execute the .dll
framework-dependent UI helpers.
581d57d
to
c0a3efa
Compare
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.
🎉 This is great! Sorry for the bad patches I sent you off-PR.. 😞
Add a new `DotnetTool` project containing the needed files to publish GCM as a cross-platform dotnet tool. Additionally, update GCM to use helper dlls if exes are not found in the Application directory. At present, dlls are only used in the dotnet tool scenario to minimize the package size and to make it cross-platform compatible.
c0a3efa
to
715eef1
Compare
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)
@ldennington Opened #1212 with measurement details |
Package GCM as a dotnet tool and publish it to NuGet.org in release pipeline.
Note 1: The pipeline is currently set up to publish to my fork of GCM using a GitHub Packages Registry. Here are the instructions to dogfood from this registry:
Note 2: We use a custom NuSpec file because, unfortunately, using
Content
in the.csproj
to copy files would not overwrite theDotnetToolSettings.xml
file. See NuGet/Home#9902 for details.Fixes #820