Cross-platform support - #518
Conversation
microsoft-github-policy-service
Bot
added
the
Issue-Feature
Ryan (sitiom)
left a comment
Ryan (sitiom)
left a comment
There was a problem hiding this comment.
Directories on Linux/macOS:
- Settings:
~/.wingetcreate/settings.json,~/.wingetcreate/settings.json.backup- Logs:
~/.wingetcreate/DiagOutputDir/<file>.txt- Installer downloads:
/tmp/wingetcreate/
It would be nice to follow the XDG Base Directory spec like most apps already do to avoid clutter in the home directory (Example: ~/.config/winget-create)
microsoft-github-policy-service
Bot
added
Needs-Author-Feedback
and removed
Needs-Author-Feedback
labels
8e6401a to
aeb188c
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ceed3ee to
d5bc8c9
Compare
ce1584e to
07a73da
Compare
b520493 to
0cb05ea
Compare
94698e3 to
072fb23
Compare
072fb23 to
a60d338
Compare
|
Flor Chacón (@florelis) can you please review this? |
Flor Chacón (florelis)
left a comment
Flor Chacón (florelis)
left a comment
There was a problem hiding this comment.
(Haven't finished reading through the whole PR yet)
Ryan (@ryfu-msft) could you also review this since you have more context? At least the sections for package parsing
Main comments so far:
- Writing the token in plain text is risky
- We need to verify the internal protocols we have for using new OSS projects
- For computing the PFN I'd like an official source of the algorithm or the ok from MSIX team. I don't want this repo to become the official MS source for how these are computed
| path = src/WingetCreateCore/Common/Msi/rust-msi\ | ||
| # TODO: Switch to mdsteele/rust-msi once the PR is merged | ||
| # https://github.com/mdsteele/rust-msi/pull/18 | ||
| url = https://github.com/vedantmgoyal9/rust-msi |
There was a problem hiding this comment.
I don't think we can just include an external repository as a submodule and then build it blindly; that seems like a security risk. At the very least, it should point to a specific commit so that we cannot get unknown, unexpected changes.
We have internal policies/procedures for evaluating new OSS we use, but I'm not that familiar with them. I need review them or ask someone else on the team..
There was a problem hiding this comment.
Pinned to a specific commit.
There was a problem hiding this comment.
I think that is still pointing to a branch, which can get newer commits at any time
There was a problem hiding this comment.
| // Generate the hash part of the package family name | ||
| var publisherSha256 = SHA256.HashData(Encoding.Unicode.GetBytes(identityNode.Attributes["Publisher"].Value)); | ||
| var binaryString = string.Concat(publisherSha256.Take(8).Select(c => Convert.ToString(c, 2).PadLeft(8, '0'))) + '0'; // representing 65-bits = 13 * 5 | ||
| var encodedPublisherId = string.Concat(Enumerable.Range(0, binaryString.Length / 5).Select(i => "0123456789ABCDEFGHJKMNPQRSTVWXYZ".Substring(Convert.ToInt32(binaryString.Substring(i * 5, 5), 2), 1))); |
There was a problem hiding this comment.
Where did this come from? If there is an official source we should link it in a comment for future reference. If not, we'll need to ask the MSIX team internally to confirm this is right and that it is appropriate for us to write publicly.
There was a problem hiding this comment.
I've added a comment referencing the source. By the way, the same algorithm is also used in YamlCreate.ps1 - https://github.com/microsoft/winget-pkgs/blob/master/Tools/YamlCreate.ps1#L853-L870.
Co-authored-by: Flor Chacón <14323496+florelis@users.noreply.github.com>
f283ecf to
04929d3
Compare
04929d3 to
8c8c70e
Compare
|
If i can use this tool on linux, that would be amazing, I have a container based workflow CI that would benefit a lot being able to publish to winget |
7 participants
win-arm64,osx-arm64,linux-arm64,linux-x64.~/.config/wingetcreate/settings.json,~/.config/wingetcreate/settings.json.backup~/.config/wingetcreate/DiagOutputDir/<file>.txt/tmp/wingetcreate/Microsoft Reviewers: Open in CodeFlow