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

The dotnet/runtime repo is now live-live! #839

Closed
jkoritzinsky opened this issue Dec 13, 2019 · 20 comments
Closed

The dotnet/runtime repo is now live-live! #839

jkoritzinsky opened this issue Dec 13, 2019 · 20 comments

Comments

@jkoritzinsky
Copy link
Member

jkoritzinsky commented Dec 13, 2019

After a few weeks of infrastructure work resulting in the merge of #494, the dotnet/runtime repository now uses live-built components between the coreclr, libraries, and installer builds. More specifically, the dotnet/runtime build now uses the results of locally building coreclr when building libraries and the results of coreclr and libraries when building installer. As a result, this will change your local workflow. When you pull down changes from the dotnet/runtime repository, we suggest that you build the project from the root build scripts in the Release configuration by running build.cmd -c Release or build.sh -c Release. This will ensure that you likely will not have to do any additional extra local builds to get started. We’ve included some guidance below for the common scenarios for various teams.

Because of the current level of control Arcade provides for configuring individual projects, the default build.sh and build.cmd scripts at the root of the repository build the entire repository in the same configuration. As a result, this workflow is likely not the most common scenario for most users. Here are some suggestions for common patterns across our teams.

CoreCLR workflow

To build the coreclr product build, your current workflow will work as expected. The tests as well as generating Core_Root now require the libraries subset category to also be built in Release. If you build the root build scripts in Release configuration as recommended, then you can build the CoreCLR tests as before. If you want to rebuild the libraries build to enable updating your Core_Root or to build CoreCLR tests against new APIs, you can run libraries.cmd -c Release or libraries.sh -c Release depending on your platform to build the updated libraries.

Libraries workflow

When working on the libraries subset, you likely don’t want to use a debug CoreCLR when testing. So, you should ensure that the CoreCLR subset you are using is a Release CoreCLR. To do so, pass /p:CoreCLRConfiguration=Release as a parameter to the libraries build script to build the libraries subset against your release build of CoreCLR.

Installer workflow

When working on the installer subset, you may want a debug or checked CoreCLR (if you work on the hosting layer), or you may want a release CoreCLR (if you work in the setup area). If you want a CoreCLR configuration that is different than your installer configuration, then pass the CoreCLRConfiguration MSBuild property to your build as the libraries workflow suggests. Additionally, if you want to use a Release version of the libraries build, you can pass the LibrariesConfiguration property in the same manner to select a Debug or Release libraries build. Due to the way that package versions are generated in the build today, building the installer subset will require the libraries subset to have been built the same day.

@davidsh
Copy link
Contributor

davidsh commented Dec 14, 2019

Libraries workflow

For those devs working primarily in the libraries, you don't have to build everything with build.cmd/build.sh. I found the following workflow useful:

Windows

src\coreclr\build.cmd -release -skiptests -clang9
libraries.cmd /p:CoreCLRConfiguration=Release

Linux/Mac

src/coreclr/build.sh -release -skiptests -clang9
./libraries.sh /p:CoreCLRConfiguration=Release

@stephentoub
Copy link
Member

Can we make libraries.cmd just default to doing that, and then allow flags to suppress/change it?

Also, libraries.cmd doesn't describe what it does. I'd personally prefer build-libraries. cmd. Or just use build.cmd -libraries, or some such thing.

@billwert
Copy link
Member

@jkoritzinsky Has this wound up in any of the workflow docs? I didn't see it there but might have missed it.

@danmoseley
Copy link
Member

@billwert i don't think so as I needed it, couldn't find it, and didn't know about this issue...

I will update them later this week, although I expect we will iterate a bit more to smooth this out.

@danmoseley
Copy link
Member

src\coreclr\build.cmd -release -skiptests -clang9

MSBUILD : error MSB1001: Unknown switch.
Switch: -clang9

@danmoseley
Copy link
Member

danmoseley commented Jan 2, 2020

Can we make libraries.cmd just default to doing that, and then allow flags to suppress/change it?
Also, libraries.cmd doesn't describe what it does. I'd personally prefer build-libraries. cmd. Or just use build.cmd -libraries, or some such thing.

I guess the proposal is

  1. delete coreclr.cmd, installer.cmd, libraries.cmd, since all they do is add -subsetcategory
  2. have flags on src\build.cmd to match (ie., -libraries as shortcut for -subsetcategory libraries, -installer for -subsetcategory installer, etc)
  3. special case -libraries / -subsetcategory libraries to also pass -p:coreclrconfiguration=release -- which can be overridden by explicitly passing -p:coreclrconfiguration=debug

is that right?

Separate question, is there a distinct "checked" configuration? Historically coreclr I believe used checked and release, and it was common at microsoft for debug to be a third option. It looks like arcade imposes "debug" and we map it to "checked" as needed? src\coreclr\build.cmd -? lists both debug and checked.

@BruceForstall
Copy link
Member

coreclr devs use both debug and checked configurations.

I'd like to see the command-line handling here be more "natural" to coreclr devs used to using src\coreclr\build.cmd (e.g., build -os arm64 from root, or src\coreclr\build.cmd arm64 -- it's annoying they're different (and IMO, the more overly verbose root build.cmd argument style is also undesirable))

@BruceForstall
Copy link
Member

@jkoritzinsky It looks like for coreclr devs to build/run debug or checked, they first need to build everything Release, then rebuild coreclr debug/checked? So, they waste all the time building coreclr Release? Can this be avoided? Or is it required to have the Release coreclr (or parts thereof) to be consumed by libraries/installer?

@billwert
Copy link
Member

billwert commented Jan 3, 2020

I also found it frustrating (though not surprising) that by building release for everything first and then just trying to build tests it all failed, because the test build defaults to not Release.

@danmoseley
Copy link
Member

@BruceForstall it's also super confusing if coreclr.cmd -c debug is equivalent to src\coreclr\build.cmd -checked (not -debug). I don't know what to do about this - Arcade presumably uses release/debug because that's what VS and most .NET projects use.

@stephentoub
Copy link
Member

stephentoub commented Jan 6, 2020

I guess the proposal is...
delete coreclr.cmd, installer.cmd, libraries.cmd, since all they do is add -subsetcategory

Yes. These are non-uniform mechanisms no one in an open source world expects. It's unintuitive to me that invoking ./libraries.sh builds stuff.

I think doing a build.cmd argumentless should build everything in the same configuration; it's otherwise confusing to know that it builds some stuff in one configuration. And presumably by default build.cmd would build everything with debug.

It'd be reasonable to have a -release/-debug/-checked set of flags, as well as -p:coreclrconfiguration (and -p:monoconfiguration once that's integrated) and -p:libraryconfiguration set of flags. Everything would default to debug and -release/-debug/-checked would change that for everything. Then -p:coreclrconfiguration and -p:libraryconfiguration would provide more fine-grained overrides. So, e.g.:

  • build.cmd => everything built as debug
  • build.cmd -release => everything built as release
  • build.cmd -checked => coreclr built as checked, libraries built as debug
  • build.cmd -release -p:coreclrconfiguration=debug => coreclr built as debug, libraries built as release
  • build.cmd -p:coreclrconfiguration=release => coreclr built as release, libraries built as debug
    Etc.

And then, yeah, if we felt the above was too onerous, we could have some common configurations behind simple flags, like build.cmd -libraries mapping to build.cmd -p:coreclrconfiguration=release.

@BruceForstall
Copy link
Member

I like the proposed simplification.

Nit: the work "configuration" seems ambiguous (and too long). I would think of "configuration" as a set of knobs all together. We've called debug/checked/release "build type" or "build flavor" before. Would it be better to use the work "type" or "flavor" instead?

The -p: syntax seems specific to msbuild; do we need to force "build.cmd" users to use that? Is the goal to not put any parsing logic into build.cmd/sh, and just "pass it on"? Typically that's meant that build.cmd/sh help messages don't include help on all the things you really need to know to get the build you want. btw, it's also annoying to use = in Windows batch script arguments because the batch processor tends to eat them. I would think something like build.cmd -checked -coreclrtype release would be simpler.

@jkotas
Copy link
Member

jkotas commented Jan 6, 2020

Everything would default to debug

I know that we are used to this default from Microsoft build environments, but I believe that this is different from the typical open source project expectations. The typical make followed by make install will give you released optimized bits. You have to go the extra mile to get debug non-optimized bits.

Since one typically wants release bits for everything and debug bits only for the component one is working on (if there is one), should we consider changing the default to Release?

@stephentoub
Copy link
Member

should we consider changing the default to Release?

Fine by me. My main concern in that statement was that everything default to the same thing.

@BruceForstall
Copy link
Member

If we change the default at the root, should we change the default in src\coreclr\build.cmd as well (and also in libraries?) to be consistent? That would have more effect on devs.

@danmoseley
Copy link
Member

danmoseley commented Jan 6, 2020

Note that also build from the root cannot currently be part of the inner loop as it is too slow (eg #1156). So for most people it will be most productive to build once from the root, and then build less as they iterate. Less might be, all of libraries, or a specific library. They need a happy path that leads them to that.

@danmoseley
Copy link
Member

@marek-safar also mentioned that his team were used to building from the root (if I understood correctly), and he found it slow - it should be easy and obvious how to build less unless and until we make it fast and incremental.

@marek-safar
Copy link
Contributor

Why would building from root be significantly slower than building from a subfolder? Building from root should not mean re-build just more checks run than when building from sub-folders.

@danmoseley
Copy link
Member

Why would building from root be significantly slower than building from a subfolder? Building from root should not mean re-build just more checks run than when building from sub-folders.

That's correct, however the incrementality of the build from the root is poor at present. This is tracked in #1156.

@safern
Copy link
Member

safern commented Jan 10, 2020

build.cmd -p:coreclrconfiguration=release => coreclr built as release, libraries built as debug
Etc.

This is currently limited by: #840 - there is a limitation from arcade to be able to build different projects in a different configuration.

The -p: syntax seems specific to msbuild; do we need to force "build.cmd" users to use that?

I agree, it would be nice to have arguments to the build script to specify which configuration/type to use for a specific partition instead of the current /p:CoreCLRConfiguration... we do that for configuration, framework in libraries, etc, we basically just translate the argument into an MSBuild parameter.

Also, it would be nice to have a smarter system. For example, if I've only built CoreCLR for Release and then I go and build libraries, it should automatically pick up that CoreCLR without the dev having to specify the CoreCLR configuration. However, if both, Release and Debug are built, it will pick whatever libraries is built against unless specified otherwise via parameters. I think we should also support automatically picking up a Checked runtime, if libraries/installer is built against Debug and a Checked CoreCLR is available in the artifacts path.

@scalablecory scalablecory unpinned this issue Jan 13, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests