Skip to content

Conversation

@chamons
Copy link
Contributor

@chamons chamons commented Mar 1, 2018

…ictions (#3387)

In addition, while writing tests for this is was noticed that mmp tests did not "really" run Release configuration correctly in most cases. Fixing this turned out to be a bit of a pain, but necessary to correctly test this (and other things).

  • Turns out that /p:configuration:debug is not sufficient to tell mmp to
    do the right thing
  • That, in most projects, sets the DebugSymbols property, which really
    is what is checked.
  • However, two of our projects did not have that, so we always did
    release mmp work.
  • Removed configuration property for tests and added real "Release"
    configuration option

…ictions (dotnet#3387)

- dotnet#3367
- App Store will now fail builds if you add in a 32-bit dylib
- If you are a 32-bit app you don't need the 64-bit part of your fat
dylib anyway
- Add --optimize=-trim-architectures to allow customization of behavior, as not everyone
uses app store

In addition, while writing tests for this is was noticed that mmp tests did not "really" run Release configuration correctly in most cases. Fixing this turned out to be a bit of a pain, but necessary to correctly test this (and other things).

- Turns out that /p:configuration:debug is not sufficient to tell mmp to
do the right thing
- That, in most projects, sets the DebugSymbols property, which really
is what is checked.
- However, two of our projects did not have that, so we always did
release mmp work.
- Removed configuration property for tests and added real "Release"
configuration option
@spouliot spouliot added this to the d15-7 milestone Mar 1, 2018
@monojenkins
Copy link
Collaborator

Build failure

@rolfbjarne
Copy link
Member

Test failures are unrelated

@rolfbjarne rolfbjarne merged commit cabc4fb into dotnet:d15-7 Mar 2, 2018
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.

4 participants