Skip to content
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

[Setup] Update wix Guids #16425

Merged
merged 5 commits into from
Feb 18, 2022
Merged

[Setup] Update wix Guids #16425

merged 5 commits into from
Feb 18, 2022

Conversation

yuyoyuppe
Copy link
Collaborator

Summary of the Pull Request

What is this about:
This PR tries to move most of the modules to one file per component model.
For justification, see the added comment at the top of Product.wxs

What is included in the PR:

  • split many multifile components
  • update Guids for components with multiple resources which make sense to keep together + add warnings so we don't forget to update Guid each time a resource is changed there. Unfortunately explicit Guid is required for Components with multiple resources.

How does someone test / validate:

Quality Checklist

  • Linked issue: [Setup] Support per-user installation #15516
  • Communication: I've discussed this with core contributors in the issue.
  • Tests: Added/updated and all pass
  • Installer: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Docs: Added/ updated
  • Binaries: Any new files are added to WXS / YML

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@yuyoyuppe
Copy link
Collaborator Author

yuyoyuppe commented Feb 17, 2022

Had to introduce some abbreviations, because WiX complains when Component/File Id is longer than 72 chars.

@microsoft microsoft deleted a comment from github-actions bot Feb 17, 2022
@github-actions
Copy link

github-actions bot commented Feb 17, 2022

@check-spelling-bot Report

🔴 Please review

See the files view or the action log for details.

Unrecognized words (1)

recyclebin

Previously acknowledged words that are now absent abap abcd abcdefgh appcontainer atop autogenerates AZCLI azurecr binskim bios btm buildcommand buildtools cameligo CCB cdpxwin cdxml CEBB CFFDAFADA ChaseKnowlden cjs CleanCodeDeveloper cljs CPPARM CPPx crutkas csx CTLCOLORSTATIC defaultcommand Deuchert dupenv DUSTIN efgh errc errorlevel estdir etcore Firefox fsscript Gamebar graphql Grayscale gsuberland hmon iccex ICONINFORMATION IMonitor INITCOMMONCONTROLSEX INSTALLLOGATTRIBUTES INSTALLLOGMODE INSTALLUILEVEL Javascript julia Knowlden kotlin ktm kts LASTEXITCODE LEQ ligo linkid litcoffee MAINICON MAKELPARAM Minimizeallwindows mkdir moderncop msiexec MSIINSTALLER MSIL namings NATIVEFNTCTL neq netlify nocache php phps plx policheck popd powerquery psc psm Pui pushd pyc pyd pyi pyo pyz Qin rda rdata rdeveen rds Recyclebin reportbug rexit robocopy scm SETRANGE SETSTEP SHAREIMAGELISTS smallbasic spamming spdth sregex STEPIT stx svh SWITCHEND symlink symlinks systemverilog taskkill tbc tcl tlbimp trackpad typeparamref UITo umd uninit UWPUI vbhtml verilog vse vsix WDK wdksetup wdkvsix We'd webclient webpack wifstream WINMSAPP WTL
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:microsoft/PowerToys.git repository
on the issue_15516_remove_guids branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/PowerToys/issues/comments/1043383096" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u
If the flagged items do not appear to be text

If items relate to a ...

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

  • binary file.

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@yuyoyuppe
Copy link
Collaborator Author

not sure what this recyclebin is about?

@github-actions
Copy link

github-actions bot commented Feb 17, 2022

@check-spelling-bot Report

🔴 Please review

See the files view or the action log for details.

Unrecognized words (1)

recyclebin

Previously acknowledged words that are now absent abap abcd abcdefgh appcontainer atop autogenerates AZCLI azurecr binskim bios btm buildcommand buildtools cameligo CCB cdpxwin cdxml CEBB CFFDAFADA ChaseKnowlden cjs CleanCodeDeveloper cljs CPPARM CPPx crutkas csx CTLCOLORSTATIC defaultcommand Deuchert dupenv DUSTIN efgh errc errorlevel estdir etcore Firefox fsscript Gamebar graphql Grayscale gsuberland hmon iccex ICONINFORMATION IMonitor INITCOMMONCONTROLSEX INSTALLLOGATTRIBUTES INSTALLLOGMODE INSTALLUILEVEL Javascript julia Knowlden kotlin ktm kts LASTEXITCODE LEQ ligo linkid litcoffee MAINICON MAKELPARAM Minimizeallwindows mkdir moderncop msiexec MSIINSTALLER MSIL namings NATIVEFNTCTL neq netlify nocache php phps plx policheck popd powerquery psc psm Pui pushd pyc pyd pyi pyo pyz Qin rda rdata rdeveen rds Recyclebin reportbug rexit robocopy scm SETRANGE SETSTEP SHAREIMAGELISTS smallbasic spamming spdth sregex STEPIT stx svh SWITCHEND symlink symlinks systemverilog taskkill tbc tcl tlbimp trackpad typeparamref UITo umd uninit UWPUI vbhtml verilog vse vsix WDK wdksetup wdkvsix We'd webclient webpack wifstream WINMSAPP WTL
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:microsoft/PowerToys.git repository
on the issue_15516_remove_guids branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/PowerToys/issues/comments/1043471959" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u
If the flagged items do not appear to be text

If items relate to a ...

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

  • binary file.

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@crutkas
Copy link
Member

crutkas commented Feb 18, 2022

pointing signing agent against this

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried the signed installer from CI, and after installing some PowerToys were broken until I installed .net framework 6.0.2 (which is pretty weird, but I'll up the installer version on another PR).
Changes look generally good, but why delete the GUID instead of doing something like GUID="*" ? That's the way to auto generate a guid on each build, right?

@jaimecbernardo
Copy link
Collaborator

Regarding auto generating GUID each time you build:
https://wixtoolset.org/documentation/manual/v3/howtos/general/generate_guids.html

@jaimecbernardo
Copy link
Collaborator

Could you please explain a bit better why this is a requirement?

add warnings so we don't forget to update Guid each time a resource is changed there. Unfortunately explicit Guid is required for Components with multiple resources.

@yuyoyuppe
Copy link
Collaborator Author

@jaimecbernardo Regarding autogenerating Ids for Components, there's a more detailed explanation here:

This value should be a guid that uniquely identifies this component's contents, language, platform, and version. If omitted, the default value is '*' which indicates that the linker should generate a stable guid. Generatable guids are supported only for components with a single file as the component's keypath or no files and a registry value as the keypath. It's also possible to set the value to an empty string to specify an unmanaged component. Unmanaged components are a security vulnerability because the component cannot be removed or repaired by Windows Installer (it is essentially an unpatchable, permanent component). Therefore, a guid should always be specified for any component which contains resources that may need to be patched in the future.

So for single file components their Guid would be auto-generated and it would be stable based on the Keypath and it should work as intended, because the file version would be different. We'll need to make sure that updating scenario works for .pngs, e.g. if you update the image itself, because they don't have a version. However, it shouldn't be an issue, since during the update, we're removing previous version's component.

More info:

I've updated PR comment to contain those links.

@github-actions
Copy link

github-actions bot commented Feb 18, 2022

@check-spelling-bot Report

🔴 Please review

See the files view or the action log for details.

Unrecognized words (1)

robmensching

Previously acknowledged words that are now absent abap abcd abcdefgh appcontainer atop autogenerates AZCLI azurecr binskim bios btm buildcommand buildtools cameligo CCB cdpxwin cdxml CEBB CFFDAFADA ChaseKnowlden cjs CleanCodeDeveloper cljs CPPARM CPPx crutkas csx CTLCOLORSTATIC defaultcommand Deuchert dupenv DUSTIN efgh errc errorlevel estdir etcore Firefox fsscript Gamebar graphql Grayscale gsuberland hmon iccex ICONINFORMATION IMonitor INITCOMMONCONTROLSEX INSTALLLOGATTRIBUTES INSTALLLOGMODE INSTALLUILEVEL Javascript julia Knowlden kotlin ktm kts LASTEXITCODE LEQ ligo linkid litcoffee MAINICON MAKELPARAM Minimizeallwindows mkdir moderncop msiexec MSIINSTALLER MSIL namings NATIVEFNTCTL neq netlify nocache php phps plx policheck popd powerquery psc psm Pui pushd pyc pyd pyi pyo pyz Qin rda rdata rdeveen rds reportbug rexit robocopy scm SETRANGE SETSTEP SHAREIMAGELISTS smallbasic spamming spdth sregex STEPIT stx svh SWITCHEND symlink symlinks systemverilog taskkill tbc tcl tlbimp trackpad typeparamref UITo umd uninit UWPUI vbhtml verilog vse vsix WDK wdksetup wdkvsix We'd webclient webpack wifstream WINMSAPP WTL
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:microsoft/PowerToys.git repository
on the issue_15516_remove_guids branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/PowerToys/issues/comments/1044406391" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u
If the flagged items do not appear to be text

If items relate to a ...

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

  • binary file.

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work!

Copy link
Collaborator

@stefansjfw stefansjfw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested. looks good

@jaimecbernardo jaimecbernardo merged commit dabf657 into main Feb 18, 2022
@yuyoyuppe yuyoyuppe deleted the issue_15516_remove_guids branch February 18, 2022 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants