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.

Sign up for GitHub

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

Jump to bottom

Ban assembly versions with wildcards in deterministic builds #5398

Merged
merged 7 commits into from
Sep 23, 2015
Merged

Ban assembly versions with wildcards in deterministic builds #5398

merged 7 commits into from
Sep 23, 2015

Conversation

TyOverby
Copy link
Contributor

Closes #4578

@dnfclas
Copy link

dnfclas commented Sep 22, 2015

Hi @TyOverby, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@TyOverby
Copy link
Contributor Author

review: @gafter @tmat @jaredpar

jaredpar
jaredpar reviewed Sep 22, 2015
View reviewed changes
src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs
@@ -2073,7 +2073,8 @@ private void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArguments<Attr
{
string verString = (string)attribute.CommonConstructorArguments[0].Value;
Version version;
if (!VersionHelper.TryParseAssemblyVersion(verString, allowWildcard: true, version: out version))
bool deterministic = _compilation.Feature("deterministic")?.Equals("true", StringComparison.OrdinalIgnoreCase) ?? false;
Copy link
Member

Choose a reason for hiding this comment

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

@gafter do we have a standard way of checking for deterministic compilation that we could reuse here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

compilation.IsEmitDeterministic exists. Don't see how that one slipped by me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 821de3c

@jaredpar
Copy link
Member

👍

@dnfclas
Copy link

dnfclas commented Sep 22, 2015

@TyOverby, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jaredpar
Copy link
Member

retest mac please

@jaredpar
Copy link
Member

test mac please

2 similar comments
@tannergooding
Copy link
Member

test mac please

@tannergooding
Copy link
Member

test mac please

TyOverby added a commit that referenced this pull request Sep 23, 2015
@TyOverby
Merge pull request #5398 from TyOverby/4578-wildcard-deterministic
af3c674 
Ban assembly versions with wildcards in deterministic builds
@TyOverby TyOverby merged commit af3c674 into dotnet:master Sep 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TyOverby @dnfclas @jaredpar @tannergooding