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

Adjust the link line for all exe/dlls to prefer onecore_apiset #922

Closed
DHowett-MSFT opened this issue May 21, 2019 · 2 comments · Fixed by #3429
Closed

Adjust the link line for all exe/dlls to prefer onecore_apiset #922

DHowett-MSFT opened this issue May 21, 2019 · 2 comments · Fixed by #3429
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@DHowett-MSFT
Copy link
Contributor

dwmapi, winmm, shlwapi are all in different forms on OneCore editions than on desktop, so linking those libs explicitly (instead of via umbrella) is probably not good.

pathcch isn't there at all... so maybe the API we're using through that is available in a different way?

Originally posted by @miniksa in #913

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 21, 2019
@DHowett-MSFT DHowett-MSFT added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. and removed Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 21, 2019
@DHowett-MSFT
Copy link
Contributor Author

Problem description: common.build.exe.or.dll.props specifies the link line, but it is included before common.build.post.props. common.build.post.props includes the toplevel C++ compilation rules which prepend the default libraries to the link line.

It is necessary to do a few things:

  • move all PropertyGroups into .props files
  • move all ItemDefinitionGroups into .targets files
  • rename ...post.props to ...post.targets, as it is a targets file that includes targets files.
  • move all .targets files that define ItemDefinitionGroups that change compilation or linking behavior to after ...post.props

I think it will benefit us to get rid of common.build.exe.props and common.build.dll.props and actually just rely on PropertyGroup.ConfigurationType being Application or DynamicLibrary to include additional rules in post.targets. They're not different enough to warrant saving one property definition that belongs in the project.

A typical project would then become:

Import common.build.props
All ClCompile Entries
All ClInclude Entries
Properties: Name, GUID, Keyword, *ProjectType*
Import common.build.targets

@miniksa
Copy link
Member

miniksa commented May 22, 2019

I am OK with this.

@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jun 19, 2019
DHowett-MSFT pushed a commit that referenced this issue Nov 4, 2019
This commit cleans up and deduplicates all of the common build
preamble/postamble across exe, dll, lib and c++/winrt projects.

The following specific changes have been made:
* All projects now define their ConfigurationType
* All projects now set all their properties *before* including a common
  build file (or any other build files)
* cppwinrt.pre and cppwinrt.post now delegate most of their
  configuration to common.pre and common.post
* (becuase of the above,) all build options are conserved between
  console and c++/winrt components, including specific warnings and
  preprocessor definitions.
* More properties that are configurable per-project are now
  conditioned so the common props don't override them.
* The exe, dll, exe.or.dll, and lib postincludes have been merged into
  pre or post and switched based on condition as required
* Shared items (-shared, -common) are now explicitly vcxitems instead of
  vcxproj files.
* The link line is now manipulated after Microsoft.Cpp sets it, so the
  libraries we specify "win". All console things link first against
  onecore_apiset.lib.

Fixes #922.
@ghost ghost added the In-PR This issue has a related PR label Nov 4, 2019
DHowett-MSFT pushed a commit that referenced this issue Nov 4, 2019
This commit cleans up and deduplicates all of the common build
preamble/postamble across exe, dll, lib and c++/winrt projects.

The following specific changes have been made:
* All projects now define their ConfigurationType
* All projects now set all their properties *before* including a common
  build file (or any other build files)
* cppwinrt.pre and cppwinrt.post now delegate most of their
  configuration to common.pre and common.post
* (becuase of the above,) all build options are conserved between
  console and c++/winrt components, including specific warnings and
  preprocessor definitions.
* More properties that are configurable per-project are now
  conditioned so the common props don't override them.
* The exe, dll, exe.or.dll, and lib postincludes have been merged into
  pre or post and switched based on condition as required
* Shared items (-shared, -common) are now explicitly vcxitems instead of
  vcxproj files.
* The link line is now manipulated after Microsoft.Cpp sets it, so the
  libraries we specify "win". All console things link first against
  onecore_apiset.lib.

Fixes #922.
DHowett-MSFT pushed a commit that referenced this issue Nov 4, 2019
This commit cleans up and deduplicates all of the common build
preamble/postamble across exe, dll, lib and c++/winrt projects.

The following specific changes have been made:
* All projects now define their ConfigurationType
* All projects now set all their properties *before* including a common
  build file (or any other build files)
* cppwinrt.pre and cppwinrt.post now delegate most of their
  configuration to common.pre and common.post
* (becuase of the above,) all build options are conserved between
  console and c++/winrt components, including specific warnings and
  preprocessor definitions.
* More properties that are configurable per-project are now
  conditioned so the common props don't override them.
* The exe, dll, exe.or.dll, and lib postincludes have been merged into
  pre or post and switched based on condition as required
* Shared items (-shared, -common) are now explicitly vcxitems instead of
  vcxproj files.
* The link line is now manipulated after Microsoft.Cpp sets it, so the
  libraries we specify "win". All console things link first against
  onecore_apiset.lib.

Fixes #922.
DHowett-MSFT pushed a commit that referenced this issue Nov 5, 2019
This commit cleans up and deduplicates all of the common build
preamble/postamble across exe, dll, lib and c++/winrt projects.

The following specific changes have been made:
* All projects now define their ConfigurationType
* All projects now set all their properties *before* including a common
  build file (or any other build files)
* cppwinrt.pre and cppwinrt.post now delegate most of their
  configuration to common.pre and common.post
* (becuase of the above,) all build options are conserved between
  console and c++/winrt components, including specific warnings and
  preprocessor definitions.
* More properties that are configurable per-project are now
  conditioned so the common props don't override them.
* The exe, dll, exe.or.dll, and lib postincludes have been merged into
  pre or post and switched based on condition as required
* Shared items (-shared, -common) are now explicitly vcxitems instead of
  vcxproj files.
* The link line is now manipulated after Microsoft.Cpp sets it, so the
  libraries we specify "win". All console things link first against
  onecore_apiset.lib.
* Fix all compilation errors caused by build unification
* Move CascadiaPackage's resources into a separate item file

Fixes #922.
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants