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

Change IBC merge to report warning, not error when IBC data are missing #2230

Closed
wants to merge 1 commit into from

Conversation

tmat
Copy link
Member

@tmat tmat commented Mar 12, 2019

This will allow us to choose whether to block the build or not when IBC data are missing for some assemblies (by enabling or disabling warnaserror for the warning).

@tmat
Copy link
Member Author

tmat commented Mar 12, 2019

@brianrob @jaredpar

@jaredpar
Copy link
Member

I actually like that this as an error. Yes our build is blocked but at the same time it's blocked for a good reasons. But agree it should be configurable.

@brianrob
Copy link
Member

By default, missing data should be an error. If you choose to try and embed data and the data is missing that should be an error. I think if you might not have data for a particular assembly and you're OK with ignoring it, then you should not call ibcmerge to begin with and should check before doing so. If we make this a warning, then its going to get abused over time and there will be assemblies out there that are inadvertently unoptimized.

Out of curiosity, what is the scenario where you want to make this a warning?

Copy link
Member

@brianrob brianrob left a comment

Choose a reason for hiding this comment

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

I'd like to understand the scenario for this. By default I believe this should be an error - if ibcmerge is being called then no data represents an error condition.

@tmat
Copy link
Member Author

tmat commented Mar 13, 2019

By default, missing data should be an error.

Arcade treats all warnings as errors by default. But I forgot that msbuild still doesn't have warnnotaserror working :( dotnet/msbuild#3062

But agree it should be configurable.

I guess we need to check NoWarn property manually :-|

Out of curiosity, what is the scenario where you want to make this a warning?

I'm not sure if not having some assemblies optimized is an error that should block the entire build.

I'm gonna close this for now. The Roslyn build is currently blocked in master, but it seems that is just a point in time issue and we should deal with that differently.

@tmat tmat closed this Mar 13, 2019
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.

3 participants