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

Updated to .NET version 4.5 #11284

Merged
merged 1 commit into from
May 20, 2016
Merged

Updated to .NET version 4.5 #11284

merged 1 commit into from
May 20, 2016

Conversation

Mailaender
Copy link
Member

Closes #10661.

@@ -3,7 +3,7 @@ Version: {VERSION}
Architecture: all
Maintainer: Paul Chote <paul@chote.net>
Installed-Size: {SIZE}
Depends: libopenal1, mono-runtime (>= 2.10), libmono-system-core4.0-cil, libmono-system-drawing4.0-cil, libmono-system-data4.0-cil, libmono-system-numerics4.0-cil, libmono-system-runtime-serialization4.0-cil, libmono-system-xml-linq4.0-cil, libfreetype6, libc6, libasound2, libgl1-mesa-glx, libgl1-mesa-dri, xdg-utils, zenity, libsdl2 | libsdl2-2.0-0, liblua5.1-0
Depends: libopenal1, mono-runtime (>= 3.2), libmono-system-core4.0-cil, libmono-system-drawing4.0-cil, libmono-system-data4.0-cil, libmono-system-numerics4.0-cil, libmono-system-runtime-serialization4.0-cil, libmono-system-xml-linq4.0-cil, libfreetype6, libc6, libasound2, libgl1-mesa-glx, libgl1-mesa-dri, xdg-utils, zenity, libsdl2 | libsdl2-2.0-0, liblua5.1-0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already in #11275. Shall I drop it from there? It makes more sense here, contextually.

Copy link
Member Author

@Mailaender Mailaender May 15, 2016

Choose a reason for hiding this comment

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

Yes, indeed. I will then rebase on top of your change of the line.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@phrohdoh
Copy link
Member

The minimum version on the installation page will need to be updated if/once this is merged.

@Mailaender
Copy link
Member Author

This is currently waiting for #11275 to get merged first.

@reaperrr
Copy link
Contributor

Needs rebase.

@Mailaender
Copy link
Member Author

Rebased.

@phrohdoh
Copy link
Member

phrohdoh commented May 18, 2016

I can build this via the Makefile (mono 4.2.3.4) and xbuild (v12.0).
The game runs fine (tested the map editor, and asset viewer in all default mods).

Some things were missed:

make.ps1:3: msBuildVersions = @("4.0")
OpenRA.Server/OpenRA.Server.csproj:61: <RequiredTargetFramework>4.0</RequiredTargetFramework>

Do any of the libmono-*4.0-cil references need to be updated? (@obrakmann)

Other than those fixes, 👍.
We're on the fast track to newer tooling. :-)

@Mailaender
Copy link
Member Author

Updated.

@@ -1,6 +1,6 @@
function FindMSBuild
{
$msBuildVersions = @("4.0")
$msBuildVersions = @("4.5")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, this needs to be reverted. Internally, 4.5 is still a sub-version of 4.0, Windows won't find MSBuild if you change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted.

@reaperrr
Copy link
Contributor

At least on my system, I get lots of CPU mismatch warnings. MSBuild defaults to Framework64, but we target x86.

The compiled code seems to work fine anyhow, but these warnings at least slow down the compile quite a bit. Not sure how to fix that, though, so I won't block the PR over this.

Maybe @penev92, @GraionDilach or @RoosterDragon have an idea how to fix this?

@GraionDilach
Copy link
Contributor

GraionDilach commented May 19, 2016

All the compilations I've seen on x64 Windows had those warnings - I'm actually surprised this PR introduces it to you. I'm aware because I accidentally had TreatWarningsAsErrors: true in the AS project file and make all ended up failing at 90% of my testers with everyone reporting the CPU mismatch warnings as the reason.

Mind you, I'm still on x86 myself so I probably couldn't reproduce it.

@RoosterDragon
Copy link
Member

You're probably seeing the same issue as in #9114.

@GraionDilach
Copy link
Contributor

Correct me if I'm wrong but all our projects are compiled to be x86, aren't they? Do we even use nonx86 libraries at all?

I'm thinking about silencing the warnings with a

    <propertyGroup>
        <ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>
            None
        </ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>
    </propertyGroup>

configuration in the project files, but it might be a bad idea.

@RoosterDragon
Copy link
Member

See #9114 (comment). If you're seeing MSB3644 in the build logs, then it can't find the reference assemblies on your machine and it's just going to go hunting in the GAC for whatever. It'll probably find the 64-bit assemblies and use those, hence the platform mismatch.

It's a build problem local to your machines - rather than any problem with the actual build config. It seems the reference assemblies need to be installed - I'm not able to suggest how to do that easily as it's always worked on my machine (I assume VS installs it).

I don't get any build problems with this 4.5 config - it seems fine :)

@reaperrr
Copy link
Contributor

Considering @RoosterDragon's comment and the fact that - apart from the warnings - this hasn't caused any real problems for me or @GraionDilach so far, I guess I can 👍 this.

@reaperrr reaperrr merged commit 60557e5 into OpenRA:bleed May 20, 2016
@reaperrr
Copy link
Contributor

changelog

@Mailaender Mailaender deleted the net-4-eol branch May 20, 2016 18:52
@reaperrr
Copy link
Contributor

reaperrr commented May 21, 2016

Edit: Nevermind, was on an outdated branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants