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

Adding msbuild.sh to the root #34272

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@Anipik
Copy link
Member

Anipik commented Dec 28, 2018

Currently we are making this file on the run and we are using the path to dotnet as Tools/dotnetcli/dotnet which no longer exists and we are not able to run the tests individually for the projects
The latest path to dotnet is from the root only i.e ./dotnet/dotnet

before

./Tools/msbuild.sh csprojpath /t:rebuildandtest (no longer working as path to dotnet has changed)

after

./msbuild.sh csprojpath /t:rebuildandtest

@Anipik Anipik assigned danmosemsft and unassigned danmosemsft Dec 28, 2018

@Anipik Anipik requested a review from danmosemsft Dec 28, 2018

@danmosemsft danmosemsft requested a review from safern Dec 28, 2018

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Dec 29, 2018

The developer documentation at https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#building-individual-projects says to get a recent dotnet onto your path and use dotnet msbuild. Why that does not work for you?

If the steps in the official developer documentation do not work, we should update them as appropriate; not just add a little hard to discover script.

@Anipik

This comment has been minimized.

Copy link
Member

Anipik commented Dec 31, 2018

I get this error while running on mac

WU2TEHPV03CX-09:corefx ani$ dotnet msbuild src/System.IO.FileSystem/src
Microsoft (R) Build Engine version 15.6.84.34536 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

/usr/local/share/dotnet/sdk/2.1.105/Sdks/Microsoft.NET.Sdk/build/Microsoft.PackageDependencyResolution.targets(327,5): error : Assets file '/Users/ani/git/corefx/artifacts/obj/System.IO.FileSystem/project.assets.json' not found. Run a NuGet package restore to generate this file. [/Users/ani/git/corefx/src/System.IO.FileSystem/src/System.IO.FileSystem.csproj]
/usr/local/share/dotnet/sdk/2.1.105/Sdks/Microsoft.NET.Sdk/build/Microsoft.PackageDependencyResolution.targets(167,5): error : Assets file '/Users/ani/git/corefx/artifacts/obj/System.IO.FileSystem/project.assets.json' not found. Run a NuGet package restore to generate this file. [/Users/ani/git/corefx/src/System.IO.FileSystem/src/System.IO.FileSystem.csproj]

I get this error while running on unix

root@c11e702a4de7:/git/corefx# dotnet msbuild src/System.IO.FileSystem/src/
bash: dotnet: command not found

But while running .dotnet/dotnet msbuild src/System.IO.FileSystem/src/ /t:rebuild
The tests and src build fine.

I will change the documentation

@Anipik

This comment has been minimized.

Copy link
Member

Anipik commented Dec 31, 2018

Earlier we used to use the Tools\DotnetCliVersion\dotnet if the dotnet is not present in the PATH Variable but now we have moved that dotnet to .\dotnet\dotnet.

I think the dotnet msbuild individualproject will still work if have appropriate version of dotnet installed on our system

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Jan 1, 2019

We had a long discussion about this in #30919 (comment) . The agreement in that discussion was that we want to make a regular global dotnet work for building corefx.

Now, I know that the corefx developer experience is not as good as it used to be after upgrading to Arcade. It has rough edges. dotnet/arcade#1364 is the worst offender for me. It would be better to fix the rough edges instead of introducing workarounds for them.

@dotnet dotnet deleted a comment from danmosemsft Jan 1, 2019

@danmosemsft

This comment has been minimized.

Copy link
Member

danmosemsft commented Jan 2, 2019

The agreement in that discussion was that we want to make a regular global dotnet work for building corefx.

Does that mean a "razzle like" batch file that adds it to the path? I don't object, but if that's the plan @Anipik can add that, instead?

@ViktorHofer

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Jan 2, 2019

Having a small helper bat/sh script that adds the local dotnet to the path - if you do not have one globally installed and you do not want to install one - sounds good to me. Also, mention it in the developer guide.

@ViktorHofer

This comment has been minimized.

Copy link
Member

ViktorHofer commented Jan 2, 2019

I get this error while running on mac

/usr/local/share/dotnet/sdk/2.1.105/Sdks/Microsoft.NET.Sdk/build/Microsoft.PackageDependencyResolution.targets(327,5): error : Assets file '/Users/ani/git/corefx/artifacts/obj/System.IO.FileSystem/project.assets.json' not found. Run a NuGet package restore to generate this file. [/Users/ani/git/corefx/src/System.IO.FileSystem/src/System.IO.FileSystem.csproj]
/usr/local/share/dotnet/sdk/2.1.105/Sdks/Microsoft.NET.Sdk/build/Microsoft.PackageDependencyResolution.targets(167,5): error : Assets file '/Users/ani/git/corefx/artifacts/obj/System.IO.FileSystem/project.assets.json' not found. Run a NuGet package restore to generate this file. [/Users/ani/git/corefx/src/System.IO.FileSystem/src/System.IO.FileSystem.csproj]

These are transient errors that should go away after a clean build.

I get this error while running on unix

You didn't install dotnet correctly. Works on my Ubuntu machine.

Having a small helper bat/sh script that adds the local dotnet to the path - if you do not have one globally installed and you do not want to install one - sounds good to me. Also, mention it in the developer guide.

We can add that if necessary, but is it really necessary? Why not use the globally installed dotnet sdk which works fine?

@ViktorHofer

This comment has been minimized.

Copy link
Member

ViktorHofer commented Jan 2, 2019

Not talking about the points that we already discussed in the past (like mixing dotnet installations, global vs repo one) but the last month showed that for a dev flow the globally installed one works just fine.

@safern

This comment has been minimized.

Copy link
Member

safern commented Jan 2, 2019

Why not just calling eng/common/msbuild.sh and passing the parameters along? That one already makes sure dotnet is installed, if not it boostraps it for you and calls the one from the right location.

@Anipik

This comment has been minimized.

Copy link
Member

Anipik commented Jan 2, 2019

You didn't install dotnet correctly. Works on my Ubuntu machine.

I am running on a docker machine and I haven't installed globally. I can try with that.

It will be helpful to have some mechanism to use the sdk from corefx repo without installying globally.
Tools\msbuild.sh script point to the the tools\dotnetcliversion\dotnet (No longer present) . cant we just redirect it to .dotnet\dotnet

@ViktorHofer

This comment has been minimized.

Copy link
Member

ViktorHofer commented Jan 2, 2019

Why not just calling eng/common/msbuild.sh and passing the parameters along?

I prefer typing dotnet msbuild instead of ~/git/corefx/eng/common/msbuild.sh. Are you a masochist? ;)

@safern

This comment has been minimized.

Copy link
Member

safern commented Jan 2, 2019

I was speaking from the script he is adding. So msbuild.sh from the root just being a wrapper of the one in eng common.

@ViktorHofer

This comment has been minimized.

Copy link
Member

ViktorHofer commented Jan 2, 2019

I still don't understand what's the purpose of adding a razzle like script over using the globally installed sdk? The only pro is that we don't mix dotnet installations when building from root vs individual projects but as already stated that scenario has been working fine for me and others for the last months so I don't see why we are changing this now?

@danmosemsft

This comment has been minimized.

Copy link
Member

danmosemsft commented Jan 2, 2019

If I build from a VS developer prompt, dotnet is already in the path, so I don't need a razzle. Do we want to make it possible to build from a clean command prompt? If so perhaps that's why there's confusion above, as in that case you would want to add it to the path.

@ViktorHofer

This comment has been minimized.

Copy link
Member

ViktorHofer commented Jan 3, 2019

Having VS installed won't be a requirement in the future anymore (dotnet/arcade#64) hence we probably want to have two scripts for both Unix and Windows. I'm not sure which workflow we should advertise in our docs as the majority of users will have dotnet installed globally anyway.

@safern

This comment has been minimized.

Copy link
Member

safern commented Jan 3, 2019

I think having an msbuild.sh/cmd script that just calls the underlying eng/common/msbuild.ps1(sh) script sounds like a reasonable thing to have as people might find shorter and easier to write msbuild.* project than .dotnet/dotnet project -- also with those scripts calling the underlying scripts, we ensure tools are installed.

Also, we could add an msbuild switch to the build scripts in the root if we don't wan to add more scripts to our repo root.

Something like build.cmd -msbuild project [parameters]

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Jan 3, 2019

Something like build.cmd -msbuild project [parameters]

build.cmd can do this already. You do not even have to specify -msbuild :

https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#building-individual-libraries

@safern

This comment has been minimized.

Copy link
Member

safern commented Jan 3, 2019

build.cmd can do this already. You do not even have to specify -msbuild :

This doesn't work great to build an individual project, as what it does is call the arcade build.ps1 script, which calls msbuild on build.proj inside arcade SDK, and then call the passed project (Plus we will always call -restore and restore the buildtools) which we only want to do once.

Also, it doesn't work great with msbuild parameters, so we should fix that if we want to encourage people using that to build a single project. Running build.cmd src\System.Collections\src\ /bl:msbuild.binlog results in:

E:\repos\corefx>build.cmd src\System.Collections\src\ /bl:msbuild.binlog
  Restore completed in 31.43 ms for D:\nuget\packages\microsoft.dotnet.arcade.sdk\1.0.0-beta.18619.2\tools\Tools.proj.
  Package Id                             Version        Commands
  ---------------------------------------------------------------------
  coverlet.console                       1.4.0          coverlet
  dotnet-reportgenerator-globaltool      4.0.5-rc2      reportgenerator
D:\nuget\packages\microsoft.dotnet.build.tasks.configuration\1.0.0-beta.18619.2\build\Microsoft.DotNet.Build.Tasks.Configuration.targets(80,5): error MSB3202: The project file "E:\repos\corefx\src\System.Collections\src /bl:msbuild.binlog/**/*.csproj" was not found. [E:\repos\corefx\src\dirs.proj]

Build FAILED.

D:\nuget\packages\microsoft.dotnet.build.tasks.configuration\1.0.0-beta.18619.2\build\Microsoft.DotNet.Build.Tasks.Configuration.targets(80,5): error MSB3202: The project file "E:\repos\corefx\src\System.Collections\src /bl:msbuild.binlog/**/*.csproj" was not found. [E:\repos\corefx\src\dirs.proj]
@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Jan 3, 2019

Yep, the documented developer experience has number of rough edges after the Arcade migration. My meta-point is that we need to make the documented developer experience to work well - by fixing both the docs and/or the scripts.

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