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

MSIX Dynamic Dependencies #393

Merged
merged 75 commits into from
Mar 9, 2021
Merged

MSIX Dynamic Dependencies #393

merged 75 commits into from
Mar 9, 2021

Conversation

DrusTheAxe
Copy link
Member

It lives! MSIX Dynamic Dependencies implemented and tested

See Issue #89 MSIX Dynamic Dependencies - allow any process to use MSIX Framework packages
and specs/dynamicdependencies/DynamicDependencies.md

This includes...

  • Dynamic Dependencies Win32 API
  • Dynamic Dependencies WinRT API
  • Bootstrapper API
  • Dynamic Dependency Lifetime Manager (DDLM)
  • Extensive test suite

Copy link
Contributor

@mikenelte mikenelte left a comment

Choose a reason for hiding this comment

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

We should not be committing pfx files. We should be able to use a signing task in the pipeline, where it can get the cert for signing. See build/MSTest.pfx

@DrusTheAxe
Copy link
Member Author

DrusTheAxe commented Jan 22, 2021

We should not be committing pfx files. We should be able to use a signing task in the pipeline, where it can get the cert for signing. See build/MSTest.pfx

This is a test cert for inner loop testing. Tests need to make signed MSIXes for some tests e.g. test\DynamicDependency\data\DynamicDependency.DataStore.Msix\Makefile.mak

Inner-loop development does not (cannot) require passing through pipeline tasks. Last thing anyone wants is F5 to build one project, then tap the build pipeline to sign a package so it can resume compiling and running test code

@DrusTheAxe DrusTheAxe closed this Jan 22, 2021
@DrusTheAxe DrusTheAxe reopened this Jan 22, 2021
@DrusTheAxe DrusTheAxe requested a review from mikenelte January 22, 2021 20:59
@mikenelte
Copy link
Contributor

We should not be committing pfx files. We should be able to use a signing task in the pipeline, where it can get the cert for signing. See build/MSTest.pfx

This is a test cert for inner loop testing. Tests need to make signed MSIXes for some tests e.g. test\DynamicDependency\data\DynamicDependency.DataStore.Msix\Makefile.mak

Inner-loop development does not (cannot) require passing through pipeline tasks. Last thing anyone wants is F5 to build one project, then tap the build pipeline to sign a package so it can resume compiling and running test code

I'm fine with this, but expect the code scanners to autoflag a private cert being checked in. You'll need to make those happy when this is flagged.

Copy link
Member

@jonwis jonwis left a comment

Choose a reason for hiding this comment

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

Got down to the tests, will review those in another pass.

dev/Detours/README.md Show resolved Hide resolved
dev/DynamicDependency/DataStore.cpp Show resolved Hide resolved
dev/DynamicDependency/DataStore.cpp Outdated Show resolved Hide resolved
dev/DynamicDependency/DataStore.cpp Outdated Show resolved Hide resolved
dev/DynamicDependency/DataStore.cpp Outdated Show resolved Hide resolved
dev/DynamicDependency/PackageGraphNode.cpp Show resolved Hide resolved
dev/DynamicDependency/PackageGraphNode.cpp Show resolved Hide resolved
dev/DynamicDependency/WinRTPackage.cpp Show resolved Hide resolved
dev/DynamicDependency/utf8.h Outdated Show resolved Hide resolved
@DrusTheAxe
Copy link
Member Author

We should not be committing pfx files. We should be able to use a signing task in the pipeline, where it can get the cert for signing. See build/MSTest.pfx

This is a test cert for inner loop testing. Tests need to make signed MSIXes for some tests e.g. test\DynamicDependency\data\DynamicDependency.DataStore.Msix\Makefile.mak
Inner-loop development does not (cannot) require passing through pipeline tasks. Last thing anyone wants is F5 to build one project, then tap the build pipeline to sign a package so it can resume compiling and running test code

I'm fine with this, but expect the code scanners to autoflag a private cert being checked in. You'll need to make those happy when this is flagged.

Thanks for the heads up. I don't know what "make those happy" entails but I'll sure I'll learn :P

The MSTest.cer+pfx are actually from some other PR or advice of @EHO-makai (?) when the old one expired last month. @EHO-makai @mikenelte were those updated in main or is the expired MSTest cert still present? If main was updated I can drop these out of my PR

… -CheckAll). Improve DevCheck help. Add 'Setup TAEF Service' to build pipeline
…atch the recent change to DevCheck's parameter syntax
… as a built-in Administrator account (Elevated and not a split token) which prevents TAEF RunAs:RestrictedUser from working. Given the CI/Pipeline running as a built-in Administrator account TAEF provides no way for us to work around it thus all AppLifecycleTests are Failed or Blocked (despite passing when run locally), and DynamicDependencies doesn't support Elevation so we can have passing tests that don't verify the actual primary target environment (DynDep, MediumIL) or failing tests that block all checkins. This problem will be solved when test execution (the whole pipeline?) is moved to Helix. When that happens we can reenable these tests.
…neration for inner-loop development needing signed MSIX. NOTE: This will be updated in a future PR to pull MSTest.pfx from the Azure Key Vault.
…Check.cmd is updated to pull MSTest.pfx from the Azure Key Vault
@DrusTheAxe DrusTheAxe force-pushed the user/drustheaxe/dyndep branch from 6c6c6ee to 9a9b3d7 Compare March 8, 2021 07:37
Copy link
Contributor

@aeloros aeloros left a comment

Choose a reason for hiding this comment

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

Please make sure to put back my the Packaged Win32 tests, as they will be enabled very soon after your changes.

test/AppLifecycle/AppLifecycle.vcxitems Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Outdated Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Outdated Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Outdated Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Show resolved Hide resolved
test/AppLifecycle/FunctionalTests.cpp Show resolved Hide resolved
@DrusTheAxe DrusTheAxe removed their assignment Mar 9, 2021
@DrusTheAxe DrusTheAxe merged commit e86588b into main Mar 9, 2021
@DrusTheAxe DrusTheAxe deleted the user/drustheaxe/dyndep branch March 9, 2021 08:04
<FunctionLevelLinking>true</FunctionLevelLinking>
<RemoveUnreferencedCodeData>false</RemoveUnreferencedCodeData>
<OmitDefaultLibName>true</OmitDefaultLibName>
<BufferSecurityCheck>false</BufferSecurityCheck>
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you've explicitly disabled BufferSercurityCheck (aka /GS https://docs.microsoft.com/en-us/cpp/build/reference/gs-buffer-security-check?view=msvc-160)?

I'm running BinSkim on our binaries, and this is getting flagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.