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

Build DafnyRuntime for netstandard2.0 and net452 instead of net6.0 #2795

Merged

Conversation

robin-aws
Copy link
Member

@robin-aws robin-aws commented Sep 23, 2022

Fixes #2779.

Another attempt after the previous attempt (#2604) had to be reverted (#2710). I tried this with a fresh clone and didn't run into the Rider problem described in #2710 - I'm relatively confident the <TargetFramework>net6.0</TargetFramework> I've removed from Directory.Build.props was to blame, and/or just needing to thoroughly clean bin and obj directories.

Note that I am partially relying on post-hoc testing of the nightly build packages after this is merged, to ensure the net452 build actually works on Windows.

Note also that the packaging warnings I refer to in one of the commit messages are an existing, orthogonal issue: #3069

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@robin-aws robin-aws marked this pull request as ready for review November 14, 2022 17:00
cpitclaudel
cpitclaudel previously approved these changes Nov 15, 2022
net452 only supports up to netstandard1.2, and that version of the standard is missing some APIs we need.

Also removed the <TargetFramework>net6.0</TargetFramework> definition from Directory.Build.props, which is most likely the reason for the previous “project.assets.json doesn't have a target for 'netstandard2.0’” errors.
@robin-aws robin-aws changed the title Build DafnyRuntime for netstandard2.0 instead of net6.0 Build DafnyRuntime for netstandard2.0 and net452 instead of net6.0 Nov 17, 2022
@robin-aws robin-aws added the run-deep-tests Tells CI to run all tests label Nov 17, 2022
Copy link
Member

@cpitclaudel cpitclaudel left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@cpitclaudel cpitclaudel left a comment

Choose a reason for hiding this comment

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

(Adding a few suggestions, but they are not worth delaying the merge over.)

@@ -127,21 +127,23 @@ def zipify_path(fpath):
"""Zip entries always use '/' as the path separator."""
return fpath.replace(os.path.sep, '/')

def run_publish(self, project):
def run_publish(self, project, framework = None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def run_publish(self, project, framework = None):
def run_publish(self, project, framework=None):

Comment on lines +138 to +140
otherArgs = []
if framework:
otherArgs += ["-f", framework]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
otherArgs = []
if framework:
otherArgs += ["-f", framework]
framework_args = ["-f", framework] if framework else []

"-o", self.buildDirectory,
"-r", self.target,
"--self-contained",
"-c", "Release"], env=env)
"-c", "Release"] + otherArgs, env=env)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"-c", "Release"] + otherArgs, env=env)
"-c", "Release", *framework_args], env=env)

Copy link
Member Author

@robin-aws robin-aws Nov 17, 2022

Choose a reason for hiding this comment

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

Ah thank you, I thought I recalled something like that. If the CI has to be rerun anyway I'll apply this (and the one above) :)

@robin-aws robin-aws enabled auto-merge (squash) November 17, 2022 18:08
@robin-aws robin-aws merged commit ca741e9 into dafny-lang:master Nov 17, 2022
robin-aws added a commit to robin-aws/dafny that referenced this pull request Nov 17, 2022
Since we can’t use the latter on .NET Standard or .NET Framework 4.5.2

Thanks, jerk who wrote for dafny-lang#2795 :)
@robin-aws robin-aws deleted the dafny-runtime-on-more-frameworks branch November 30, 2022 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-deep-tests Tells CI to run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DafnyRuntime not compatible with .NET frameworks earlier than .NET 6.0
3 participants