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

Sign UWP packages by default. #25661

Closed
wants to merge 2 commits into from
Closed

Sign UWP packages by default. #25661

wants to merge 2 commits into from

Conversation

@jdm
Copy link
Member

jdm commented Jan 31, 2020

Signing needs to be disabled when creating packages that are submitted to the MS app store, but it needs to be enabled in order to create packages that can be sideloaded. Since sideloading is the much more common operation, let's turn it on by default and make it possible to opt out of it.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25362
  • These changes do not require tests because we don't have UWP tests
@highfive
Copy link

highfive commented Jan 31, 2020

Heads up! This PR modifies the following files:

@Manishearth
Copy link
Member

Manishearth commented Jan 31, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 31, 2020

📌 Commit 6a7f26f has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Jan 31, 2020

Testing commit 6a7f26f with merge 743e627...

bors-servo added a commit that referenced this pull request Jan 31, 2020
Sign UWP packages by default.

Signing needs to be disabled when creating packages that are submitted to the MS app store, but it needs to be enabled in order to create packages that can be sideloaded. Since sideloading is the much more common operation, let's turn it on by default and make it possible to opt out of it.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362
- [x] These changes do not require tests because we don't have UWP tests
@bors-servo
Copy link
Contributor

bors-servo commented Jan 31, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Jan 31, 2020

         C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(4353,5): error APPX0102: A certificate with thumbprint '45A22CA001C9C6BE412BF449FBE70FEDE635831F' that is specified in the project cannot be found in the certificate store. Please specify a valid thumbprint in the project file. [C:\Users\task_1580457049\repo\support\hololens\ServoApp\ServoApp.vcxproj]
         C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(4353,5): error APPX0107: The certificate specified is not valid for signing. For more information about valid certificates, see http://go.microsoft.com/fwlink/?LinkID=241478. [C:\Users\task_1580457049\repo\support\hololens\ServoApp\ServoApp.vcxproj]
@Darkspirit
Copy link
Contributor

Darkspirit commented Jan 31, 2020

@jdm
Copy link
Member Author

jdm commented Jan 31, 2020

Good catch!

@jdm jdm force-pushed the jdm:hololens-signing branch from 6a7f26f to 7686c77 Feb 12, 2020
@jdm
Copy link
Member Author

jdm commented Feb 12, 2020

@bors-servo try=windows

@bors-servo
Copy link
Contributor

bors-servo commented Feb 12, 2020

Trying commit 0bee606 with merge 01e799b...

bors-servo added a commit that referenced this pull request Feb 12, 2020
Sign UWP packages by default.

Signing needs to be disabled when creating packages that are submitted to the MS app store, but it needs to be enabled in order to create packages that can be sideloaded. Since sideloading is the much more common operation, let's turn it on by default and make it possible to opt out of it.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362
- [x] These changes do not require tests because we don't have UWP tests
@bors-servo
Copy link
Contributor

bors-servo commented Feb 12, 2020

💔 Test failed - status-taskcluster

bors-servo added a commit that referenced this pull request Feb 13, 2020
make sure UWP app can run

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Feb 13, 2020
make sure UWP app can run

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
@paulrouget
Copy link
Contributor

paulrouget commented Feb 14, 2020

Cannot import the key file 'ServoApp_TemporaryKey.pfx'. The key file may be password protected. To correct this, try to import the certificate manually into the current user's personal certificate store.

Running into that issue as well: #25745

I think that not using a password will solve that specific issue, but installing the certificate (in the Root store) will solve that issue and also the issue we have in #25745.

I don’t know if having a password will cause problems after the certificate has been installed.

@SimonSapin can you help: we need to run:

certutil.exe -addstore "Root" servo/support/hololens/ServoApp/ServoApp_TemporaryKey.pfx

bors-servo added a commit that referenced this pull request Mar 3, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 3, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 4, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
@paulrouget paulrouget mentioned this pull request Mar 9, 2020
3 of 3 tasks complete
bors-servo added a commit that referenced this pull request Mar 9, 2020
Properly sign UWP package

Supersede #25661
Fix #25362

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362 (GitHub issue number if applicable)
bors-servo added a commit that referenced this pull request Mar 9, 2020
Properly sign UWP package

Supersede #25661
Fix #25362

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362 (GitHub issue number if applicable)
bors-servo added a commit that referenced this pull request Mar 9, 2020
Properly sign UWP package

Supersede #25661
Fix #25362

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362 (GitHub issue number if applicable)
bors-servo added a commit that referenced this pull request Mar 9, 2020
Properly sign UWP package

Supersede #25661
Fix #25362

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25362 (GitHub issue number if applicable)
@bors-servo
Copy link
Contributor

bors-servo commented Mar 9, 2020

The latest upstream changes (presumably #25925) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm jdm closed this Mar 9, 2020
bors-servo added a commit that referenced this pull request Mar 14, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 17, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 18, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 20, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 20, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 27, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 27, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Mar 31, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Apr 1, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Apr 1, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
bors-servo added a commit that referenced this pull request Apr 6, 2020
make sure UWP app can run

Fix #25718

This includes #25661 with some minor tweaks (certificate file reference was missing, and its id was wrong) and a python script.

Still need to hook that to our CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.