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

[MicroWin] Fix Recall "Dependency" Misinformation #2947

Conversation

CodingWonders
Copy link
Contributor

@CodingWonders CodingWonders commented Oct 18, 2024

Type of Change

  • Bug fix

Description

This PR removes the Recall exclusion and adds the true fix for #2697. Thanks to @WitherOrNot and @thecatontheceiling for spotting this problem, and thanks to Microsoft for not testing Jack.

Testing

Testing has concluded with no issues

Impact

Clarify that it's a Windows bug. This should not have any impact on end-users.

Issue related to PR

  • Resolves something external from GitHub - any misinformation caused by Recall being a "dependency"

Additional Information

Please test this for any issues.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no errors/warnings/merge conflicts.

Fixed the misinformation caused by the Recall feature. Thanks @WitherOrNot and @thecatontheceiling for spotting the problem
Comment on lines +180 to +185
# 3. Open the file and do the modification
$appxManifest = Get-Content -Path "$scratchDir\Windows\SystemApps\MicrosoftWindows.Client.FileExp_cw5n1h2txyewy\appxmanifest.xml"
$originalLine = $appxManifest[13]
$dependency = "`n <PackageDependency Name=`"Microsoft.WindowsAppRuntime.CBS`" MinVersion=`"1.0.0.0`" Publisher=`"CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US`" />"
$appxManifest[13] = "$originalLine$dependency"
Set-Content -Path "$scratchDir\Windows\SystemApps\MicrosoftWindows.Client.FileExp_cw5n1h2txyewy\appxmanifest.xml" -Value $appxManifest -Force -Encoding utf8
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if modifying the app manifest through line numbering is a good idea.. but I can't tell/know a better approach to fix this problem, other then pattern matching using RegEx.. which'll require a bit testing, and some headaches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've used this method because I had used it earlier for DT (more on that here) and it works there.

I can't think of another way to insert a line on PowerShell either.

Copy link

@amirulshukry amirulshukry Oct 24, 2024

Choose a reason for hiding this comment

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

Can I suggest this?

# Set file name
$File = '.\appxmanifest.xml'

# Process lines of text from file and assign result to $NewContent variable
$NewContent = Get-Content -Path $File |
    ForEach-Object {
        # Output the existing line to pipeline in any case
        $_

        # If line matches regex
        if($_ -match ('^' + [regex]::Escape('        <TargetDeviceFamily Name="Windows.Desktop" MinVersion="10.0.21340.0" MaxVersionTested="10.0.32767.0" />')))
        {
            # Add output additional line right after it
            '        <PackageDependency Name="Microsoft.WindowsAppRuntime.CBS" MinVersion="1.0.0.0" Publisher="CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US" />'
        }
    }

# Write content of $NewContent back to file
$NewContent | Out-File -FilePath $File -Encoding Default -Force

from https://superuser.com/questions/821112/add-line-to-text-file-after-specific-line-with-powershell

Applied here as

                    # 3. Open the file and do the modification
                    $appxManifest = Get-Content -Path "$scratchDir\Windows\SystemApps\MicrosoftWindows.Client.FileExp_cw5n1h2txyewy\appxmanifest.xml" |
					ForEach-Object {
						# Output the existing line to pipeline in any case
						$_

						# If line matches regex
						if($_ -match ('^' + [regex]::Escape('        <TargetDeviceFamily Name="Windows.Desktop" MinVersion="10.0.21340.0" MaxVersionTested="10.0.32767.0" />')))
						{
							# Add output additional line right after it
							'        <PackageDependency Name="Microsoft.WindowsAppRuntime.CBS" MinVersion="1.0.0.0" Publisher="CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US" />'
						}
					}
                    Set-Content -Path "$scratchDir\Windows\SystemApps\MicrosoftWindows.Client.FileExp_cw5n1h2txyewy\appxmanifest.xml" -Value $appxManifest -Force -Encoding utf8

Copy link
Contributor

@og-mrk og-mrk left a comment

Choose a reason for hiding this comment

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

Other then the suggestion in this review comment, these changes looks good to me, well done @CodingWonders 👍

@CodingWonders
Copy link
Contributor Author

I don't want to deal with Recall ever again... doing that was as awful as dealing with News and Interests on Windows 10 (which I eventually failed at for the anniversary PR)

@thecatontheceiling
Copy link

This change can get reverted by a windows update but it's fine since this is only necesarry during windows installation, if the change gets reverted after that then nothing will break

@ChrisTitusTech ChrisTitusTech merged commit bfaba14 into ChrisTitusTech:main Oct 24, 2024
1 check passed
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.

5 participants