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

Per-app #defines #256

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Per-app #defines #256

merged 2 commits into from
Aug 2, 2022

Conversation

CarlosNihelton
Copy link
Collaborator

This PR implements a mechanism to let MSBuild set some #defines customised per distro version (i.e. per appx version built).

It relies on the wsl-builder component to apply a file named Preprocessor.props from meta/ subtree according to the appx version being built during the build system preparation, so each Ubuntu app can have its own version of that file with its own list of #defines.

We can later use that mechanism to make MSBuild #define WIN_OOBE_ENABLED for UbuntuPreview and that effectively selects the correct app strategy for the OOBE. The file meta/**/Preprocessor.props must set ItemDefinitionGroup.ClCompile.PreprocessorDefinitions to a list of values that will be passed to the compiler as if the command line /D<VALUE> was passed such as illustrated below:

diff --git a/meta/UbuntuPreview/src/DistroLauncher/Preprocessor.props b/meta/UbuntuPreview/src/DistroLauncher/Preprocessor.props
new file mode 100644
index 0000000..f791092
--- /dev/null
+++ b/meta/UbuntuPreview/src/DistroLauncher/Preprocessor.props
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="https://schemas.microsoft.com/developer/msbuild/2003">
+       <ItemDefinitionGroup>
+               <ClCompile>
+                       <PreprocessorDefinitions>WIN_OOBE_ENABLED</PreprocessorDefinitions>
+               </ClCompile>
+       </ItemDefinitionGroup>
+</Project>

Notice that a default DistroLauncher/Preprocessor.props is not allowed and it's git-ignored. Default #defines must be visible in the source code. That file is for appx-version-specific customisations, thus only those inside meta/ are meant to be version-controlled.

To ensure that the constant definition won't be too early or too late a common Directory.Build.props was placed inside DistroLauncher/ forcing the Preprocessor.props to be imported before common Cpp props (if that file exists, of course).
That required changes in the CI QA workflow, because it was based on placing a custom Directory.Build.props file inside DistroLauncher/ directory to override the ClangTidy MSBuild target. The same behavior can be achieved with a .targets file even though it's loaded later in the build process than the .props file.

This PR just implements the mechanism. The per-app define files must be later created as needed in their respective meta/ subdirectories.

To avoid conflicts with the Preprocessor.props import mechanism.
By setting them in the Preprocessor.props per distro version.
DistroLauncher\Directory.Build.props should not change.
@CarlosNihelton CarlosNihelton marked this pull request as ready for review July 29, 2022 16:36
@CarlosNihelton CarlosNihelton merged commit c95c6f3 into main Aug 2, 2022
@CarlosNihelton CarlosNihelton deleted the per-app-defines branch August 2, 2022 11:23
CarlosNihelton added a commit that referenced this pull request Sep 13, 2022
As before, should Jammy get the niceties of the new pipeline backported,
we must move this from meta to the default build tree.
This builds on top of #256 to allow
per app #define's.
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.

2 participants