Skip to content

Set better defaults for references and project references to keep MdMerge happy. #612

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

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

jlaanstra
Copy link
Contributor

@jlaanstra jlaanstra commented May 7, 2020

Current released versions of MdMerge don't like duplicate definitions. This PR attempts to reduce the duplication by not copying reference output to the output folder by default unless this is an application. An application is always a leaf node and its winmd should never be merged into something else anyway.

Overrides are still possible since this is defined in the props file which is imported very early in the build process, to allows folks to change the default if needed. Unit test projects are an example where this might be needed, but there is not a good way to detect them.

A non-C++/WinRT project won't get these defaults and those projects might still require manually setting Private=false, if it is reference by a C++/WinRT project, so this solution isn't perfect but it helps somewhat.

TODO:

  • Test razzle build

Fixes #452

@jlaanstra jlaanstra requested review from kennykerr and Scottj1s May 7, 2020 20:28
@jlaanstra
Copy link
Contributor Author

/azp build

@azure-pipelines
Copy link

Command 'build' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@jlaanstra
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@@ -43,6 +43,20 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<ProxyFileName Condition="'%(Midl.ProxyFileName)'==''">nul</ProxyFileName>
<TypeLibraryName Condition="'%(Midl.TypeLibraryName)'==''"></TypeLibraryName>
</Midl>
<ProjectReference Condition="'$(XamlLanguage)' != 'C++'">
Copy link
Member

Choose a reason for hiding this comment

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

these are defaults. if necessary, users could explicitly override in the IDE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. Overrides are possible per reference, for all references or conditionally.

@kennykerr
Copy link
Collaborator

I like the "to keep MdMerge happy" part. 😉

@jlaanstra jlaanstra changed the title Set better default for references and project references to keep MdMerge happy. Set better defaults for references and project references to keep MdMerge happy. May 7, 2020
@ChrisGuzak
Copy link
Member

leave node

leaf node

@kennykerr
Copy link
Collaborator

What's the status of this PR?

@jlaanstra
Copy link
Contributor Author

@kennykerr I need to test the razzle build. Will try to do that tomorrow.

@jlaanstra
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

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.

MDM2009 duplicate type error when using a grandchild reference
4 participants