Conversation
f4a9629 to
aa67f47
Compare
42f9718 to
8f9cff5
Compare
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Signed-off-by: Shmueli Englard <shmueli.yosef@englard.net> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
The CI fails because winget is not available on GitHub Actions runners. Instead of installing Microsoft.WinAppCLI via winget and invoking makeappx.exe through the winapp wrapper, find and use makeappx.exe directly from the pre-installed Windows SDK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
MSYS2 automatically converts forward-slash arguments like /v into Windows paths (e.g. V:/), which causes makeappx.exe to fail with 'Unknown command line option'. Set MSYS_NO_PATHCONV=1 to disable this conversion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
The LIST entries are relative to the MSYS root (/). When makeappx.exe runs, it resolves them relative to the current working directory instead. Convert them to absolute Windows paths using the MSYS root so makeappx.exe can find the files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
Using sed with Windows paths in the replacement string causes backslash sequences like \a and \b to be interpreted as special characters, mangling the paths. Use a shell while-read loop instead to avoid escaping issues entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
MSIX packages require versions in strict X.X.X.X numeric format. Convert Git for Windows versions (e.g. '2.47.1.windows.1' -> '2.47.1.1') and handle test versions (e.g. '0-test' -> '0.0.0.0') by stripping non-numeric characters and padding with .0 segments as needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Shmueli Englard <senglard@microsoft.com>
8f9cff5 to
9e28cd5
Compare
dscho
left a comment
There was a problem hiding this comment.
Hi @shmuelie,
Thank you for working on this. MSIX packaging for Git for Windows is something that was attempted a couple of times, and I appreciate you picking it up. That said, this PR needs significant rework before I can consider merging it. Let me walk through the concerns.
Commit history. In Git for Windows we polish commits to tell a clean, compelling story. The current history reads like a lab notebook: "Initial prototype for MSIX creation", "Minor cleanup", "Initial release script", "Fix generated package name", "Fix manifest", followed by five Copilot-assisted fixup commits. Errors should be squashed into the commits that introduced them, not stacked on top. Please rework the branch into a small number of logical, self-contained commits that each do one thing correctly. For example, one commit could refactor portable/release.sh to separate out reusable parts, and the next one could add a minimal manifest template with the release.sh script to build an MSIX, another the asset generation (see below). Each commit message should explain why, not just what.
Committed image assets. The PR adds 8 PNG files under msix/Assets/. I would much rather see a small automation step that generates these from the git-for-windows.png (or git-for-windows.svg) already in the repo root at build time, resizing to the required dimensions. I notice that several of the committed PNGs share the same blob hash (3 files are identical at b5c5143, 2 more at fa651cf), which reinforces that these are mechanically derived. Committing generated binary artifacts that cannot easily be manually recreated or verified is something I try to avoid. If generating them at build time is truly not feasible, that needs justification. You could then still follow the example in gitforwindows.org where generated files are committed, but are accompanied with easy, reproducible steps to recreate them.
Configurability. Previous attempts at MSIX/Store packaging ran into a fundamental problem: the Git for Windows installer allows extensive configuration (default editor, line endings, PATH modifications, credential helper, terminal emulator, etc.), but an MSIX package has no such configuration UI. The script currently hardcodes credential.helper manager and core.fscache true into the system gitconfig, which is a reasonable starting point, but the PR description needs to address this gap explicitly. What defaults are baked in? How can users change them conveniently after installation and persist the changes across upgrades? The manifest declares desktop6:FileSystemWriteVirtualization and desktop6:RegistryWriteVirtualization as disabled, and requests unvirtualizedResources. Does that mean the packaged etc/gitconfig is writable by the user post-install? This needs to be documented.
Missing PR description. The PR currently has no description at all. It needs to explain at minimum: How to build the MSIX artifact (the exact commands; note that the script's opening comment still says "Build the portable Git for Windows", suggesting it was copy-pasted from portable/release.sh without updating). How to code-sign the resulting .msix, or how to install it without signing (MSIX packages must be signed, or the user must enable Developer Mode). How to actually use the result: does it work? What does the user experience look like? Does git-bash.exe launch correctly from the Start Menu? Whether the execution aliases (git.exe, bash.exe, sh.exe, nano.exe) clash with a regular Git for Windows installation on the same machine. Registering bash.exe and sh.exe as app execution aliases seems particularly aggressive for users who have WSL or Cygwin installed. Whether the MSIX package can provide a central Git config analogous to C:\Program Files\Git\etc\gitconfig (which Git for Windows' installer provides, and which MinGit picks up so that tools like Visual Studio, VS Code, and SmartGit can rely on the central file for shared configuration).
Relationship to existing stale msix/ content. The msix/ directory already contains appxmanifest.xml, PackagingLayout.xml, Microsoft.Alm.Authentication.dll, WebView2Loader.dll, and stale .appx/.appxbundle binaries from a previous abandoned attempt (dating back to the Git 2.32 era). This PR adds a new appxmanifest.xml.in alongside the existing appxmanifest.xml without removing or replacing the old files. This needs to be cleaned up, and that cleanup deserves its own commit at the start of the series with a brief explanation of what the old files were and why they are being replaced.
Publisher identity. The new appxmanifest.xml.in uses Publisher="CN=The Git Development Community" and Name="Git.GitforWindows", while the old appxmanifest.xml uses Publisher="CN=Johannes Schindelin, ..." and Name="JohannesSchindelin.Git". The publisher string in an MSIX must match the signing certificate exactly, so this is not a cosmetic choice. Can you explain the rationale for the new identity?
I am genuinely impressed by the effort you put in, and want to assist so that you can succeed. The approach of reusing make-file-list.sh and basing this on the MinGit/portable infrastructure looks right. But the PR needs the rework described above: clean commit history, following the DRY principle, generated (ideally not committed) assets, cleanup of the stale files, and most importantly a thorough PR description that addresses the configurability and coexistence questions.
Thank you!
|
This PR adds code for building an MSIX on x86, amd64 and arm64. I don't think we'll ever want to publish a MSIX for x86, though.
I also have questions about these aliases. |
|
Awesome, thank you for the feedback. I was honestly so overjoyed to get something that works I forgot to think about other things 😅. Hope to take a look at each point tonight and get back to you! |
|
Commit history: Happy to do that, and yeah... you can clearly see the history of me trying to get it working. Will rework the history as I work on the other issues Committed image assets: They were generated from the SVG as you suggested. The tool I used is not free or open source so I wouldn't want to add it to the build pipeline. I will look for a way to generate them in the pipeline though Configurability: There are a couple of options here. MSIX packages can have locations that are user editable to allow configuring the system level configuration, though that is not what the feature you mentioned are about. Those features are so that the applications in the package don't see a virtualized version of the file system. As for the PATH editing, that would not be needed as that's why the aliases are for. Aliases can be turned on and off in Windows Settings, allowing users to choose which are enabled from the package. Missing PR description: Very good points. I will edit the PR description and the script. Thought I had changed all the messages but guess I missed a few 😅 Relationship to existing stale Publisher identity: I just put values there so I would have something to work with, very likely we'd have to change those. As for the questions about the various alias: In the packaged world, you don't modify the PATH, instead for every application you can create one or more aliases that have kind of symbolic links created for them. As I noted above, each alias can be individually turned on or off to help the user select what will be found. This can even work with multiple packages declaring the same aliases. |
Adds (or at least attempts to add) a MSIX installer for Git based on the portable installer. The generated package adds entries to the start menu and supports running commands from command line.