Moving Rover (dotnet-bootstrapping tool) from private repository into dotnet/core #326

Merged
12 commits merged into from Nov 29, 2016

Conversation

Projects
None yet
5 participants
@ghost

ghost commented Nov 1, 2016 edited by ghost

Tool itself is functional, but need to build the testing pieces.

Going to start a list here, we've got good discussions and feedback happening, so I thought I'd just keep the work centralized here so that everyone is in the loop as to what I am up to (I click the checkbox as I accomplish the work - pushes will be intermittent throughout the day; order is determined by mood):

  • Explicitly copy just the binaries listed in the target repository directory (at the moment I am using the Kleene Star to select all the so files to copy) [ I'd like to add in a 'patching' feature that will just adopt the layout of the target directory and then move files from a source path; not this week though]
  • Incorporate Run script for the docker containers (i.e. something that will just launch them and execute my test case)
  • Use DotNet Bot + QOTD as test case
  • Incorporate intended Docker scripts
  • Provide examples in the README.md
  • Document the layout of the working directory
  • Do a 'satisfaction pass' over the documentation to make it... cleaner. [ used format of man pages ]
  • Place .gitattributes/.gitignore in root (if not there); crib (great word) from CoreFX
  • Remove dependency installer
  • (Regarding colored text) Guard writing the escape codes with a check for sys.stdout.isatty()
  • (Minor bug fix) - On Unexpected Rover Exception I am still printing a success message (oops) [ added an os._exit(...) call to the UnexpectedRoverException() method so that after we catch the exception we just bail out. This should prevent any messages from popping up or risk of some future addition executing that maybe depends on this existing ]
  • (Minor bug fix) - If interrupted during the cloning of repositories, restarting causes us to skip over the clone step [ I removed the -clone switch, I think this was over-engineered so I just check if the directory exists, if it doesn't then I will clone ]
  • Add a -to that overrides the default directory name that we stage in. Currently we stage in a local directory with the format 'dotnet-'
  • Cache CLI download
  • sudo repro-script on creation (so long as uid == 0)

ellismg was assigned Nov 1, 2016

tools/cli-bootstrap/.gitattributes
@@ -0,0 +1,3 @@
+*.py -crlf
@ellismg

ellismg Nov 1, 2016

Owner

Why do we have to go out of our way to set line endings here? Won't git just do the right thing on windows system if you have core.autocrlf set (which is the default)?

@ghost

ghost Nov 1, 2016 edited

Revised - lets remove it, I think that my concern is alleviated by the CI system we have coming up.

@ellismg

ellismg Nov 2, 2016 edited

Owner

We could consider just doing this:

###############################################################################
# Set default behavior to automatically normalize line endings.
###############################################################################
* text=auto

in a top level .gitattributes (that's what most other dotnet projects do).

@ghost

ghost Nov 2, 2016

Cool! Lets do that (if we don't have it already, ill check)

tools/cli-bootstrap/README.md
+up the build, make changes, etc. This is 'development mode' or DevMode for short. In DevMode, NO git commands are executed. This is to prevent
+the script from stomping out any changes you have made in the working directory.
+
+DevMode is triggered if the working directory is pre-existing. Consequently, when things fail in the script, we do not 'clean up' the working directory. We write a repro script
@ellismg

ellismg Nov 2, 2016

Owner

Maybe you can put an example here of what the working directory is?

@ghost

ghost Nov 2, 2016

Ah yes. I will revise the docs a bit overall. There is a bunch of stuff here that I do not like or is even a bit out-dated already.

tools/cli-bootstrap/README.md
+## Work Flow
+Place Rover in a directory. Run it.
+
+In the event of a failure, we prevent clean-up. If the failure is not a rover script error, there is likely a rover_failure-repro.sh in the directory where the command failed (you are notified on
@ellismg

ellismg Nov 2, 2016

Owner

Might be nice to document the layout of the working directory and where artifacts are once a build is complete.

tools/cli-bootstrap/cli.bootstrap.py
+ PatchTarget_SDK = ''
+ PatchTarget_Host = ''
+
+ DotNetCommitHash = 'rover-boot-strap'
@ellismg

ellismg Nov 2, 2016

Owner

Are we unable to flow the commit hash from the program we are patching? We should already have it because we checked out the correct SHA to build, right?

@ghost

ghost Nov 2, 2016 edited

This variable is changed later on - this is the 'initial value'.

@ellismg

ellismg Nov 2, 2016

Owner

Gotcha. Sorry.

@@ -0,0 +1,39 @@
+#!/usr/bin/env python
@ellismg

ellismg Nov 2, 2016

Owner

Can we just remove all this stuff? I don't think it's super worthwhile. I don't know of another project that tries to automagically install a tool-chain on your behalf. I also worry that we'll just constantly be adding packages for different things. We should just document the toolchain at a high level.

@@ -0,0 +1,8 @@
+FROM debian:8.0
@ellismg

ellismg Nov 2, 2016

Owner

It's by design that these docker files don't have toolchains installed right?

@ghost

ghost Nov 2, 2016

Yes, I will be 'borrowing' from the CLI dockers in a later PR.

@mellinoe

mellinoe Nov 2, 2016

Member

Should these dockerfiles just be removed for now, then? It doesn't even look like most of them will actually build at all since they're all using the same Ubuntu commands.

@ghost

ghost Nov 2, 2016

I don't see how having these here causes a problem other than personal preference.

@mellinoe

mellinoe Nov 2, 2016

Member

I was just suggesting that we add them with the follow-up PR instead, since a lot of these don't actually work in their current state (they all use apt-get commands). It's not a big deal either way.

@ghost

ghost Nov 2, 2016

Actually, since there is more feedback than I anticipated, I will probably update this PR tomorrow with the intended Docker scripts; so, stay tuned.

@@ -0,0 +1,8 @@
+FROM debian:8.0
+MAINTAINER Bryan P. Arant <bryanar@microsoft.com>
+RUN apt-get -qqy update
@ellismg

ellismg Nov 2, 2016

Owner

Consider doing all this setup in one run command to reduce the number of layers that are constructed. Also consider cleaning the apt-get cache as part of the same command to reduce layer size (the dockerfiles that we have for dotnet core should do this if you need an example).

Owner

ellismg commented Nov 2, 2016

It may make sense to move .gitignore and .gitattributes to the top level of the repository (and maybe just crib from the version we have in corefx)?

tools/cli-bootstrap/cli.bootstrap.py
+
+# ROVER BASE #
+class RoverMods:
+ HEADER = '\033[95m'
@mellinoe

mellinoe Nov 2, 2016

Member

Does this stuff work in docker containers? I'm pretty wary of us putting any colors into stuff now, since our attempts elsewhere rarely work in docker, and often break basic text output and formatting.

@ghost

ghost Nov 2, 2016 edited

Yep, works fine in my experience. I've never seen a problem with this kind of thing before, do you have a specific example?

@ellismg

ellismg Nov 2, 2016

Owner

Can you guard writing these escape codes with a check for sys.stdout.isatty()? That will prevent them from being written when output is redirected (which folks may want to do while debugging).

@mellinoe

mellinoe Nov 2, 2016 edited

Member

I just tested and this works on an alpine container for me. core-setup and cli have their own variant of console color, and it doesn't work in most docker containers. There's also some shell scrpts around that don't work unless you install stuff that they use for console coloring.

But this works, so I'm cool with it. 👍

tools/cli-bootstrap/cli.bootstrap.py
+
+ # replace native files in 'shared' folder.
+ # from coreclr
+ RoverShellCall('cp *so %s'%(RoverSettings.PatchTarget_Shared), cwd=coreclr_bin_directory)
@mellinoe

mellinoe Nov 2, 2016

Member

We don't need all of the *so files from coreclr's output, only the ones that should be in the shared framework directory.

tools/cli-bootstrap/cli.bootstrap.py
+ core_setup_working_git_directory = path.join(RoverSettings._srcDirectory, 'core-setup')
+ libuv_working_git_directory = path.join(RoverSettings._srcDirectory, 'libuv')
+
+ default_coreclr_bin_directory = '%s/bin/Product/Linux.x64.Release/'%(coreclr_working_git_directory)
@mellinoe

mellinoe Nov 2, 2016

Member

Is there a way to override this path? I think FreeBSD uses a different path, for example. Ditto for the next line.

@ghost

ghost Nov 2, 2016 edited

Yeah, I kind of want to see the build system overall stabilize there. Isn't there an API or something coming along in this regard? Otherwise this tool will have to chase the repositories behaviors around everywhere (as it seems we do anyways).

Edit: Sorry moving around a lot right now, I re-read this and it doesn't make much sense. What I mean is - It'd be nice if the repositories had a uniform place they would place the binaries. For example, it'd be nice if I could just reliably depend on /bin/ instead of /bin/product/SomeSpecificFolderForASpecificBuild where I have to deduce different configurations before even building the binaries. In other words - I dont want this tool to have to figure out the internals of the repository it is trying to build. We should instead strive to make the repositories build script less 'leaky'. Does this make sense?

@ghost

ghost Nov 2, 2016 edited

@ellismg can you give some perspective on this too? I really do not have much of a clue after further reflection about the best way to handle the case @mellinoe is bringing up. I could just raise up a config for this, but I do not want that to become 'the way' we do things. Is that unreasonably picky?

Edit: Actually, here is an idea, require in the dev tooling that being able to specify where the binaries will be placed (like in coreclr, build.sh --bindir wherever/i/want) - then this way we can side-step the 'figure out which directory it will be in' problem. I can then place the binaries in the _WorkingDirectory/_objDirectory/{repo}. I'll check tomorrow whether the repositories have support for this across the board.

@mellinoe

mellinoe Nov 3, 2016 edited

Member

EDIT: Oops, didn't see you commented again; this one is in response to the first reply above.

Yes, it makes sense. It's a difficult thing to solve unless the coreclr build tells you in some way where it is putting the binaries. The other problem with coding the logic here in this script is that, if the logic changes in coreclr, then the script here will work for some coreclr commits and not others.

In my script, I essentially pattern-matched the output location of CoreCLR from the build log. That is fragile in its own way, but it's also sort of similar to other pattern matching stuff in this script (to determine the commit hash). And it lets you be resilient to changes between commits, as long as the build output format doesn't change drastically. I don't know if that is a great, robust solution, either, but it's something to consider.

@ghost

ghost Nov 3, 2016 edited

I might steal your method Eric :D... I want to investigate a few things first before I do that, also I have one more idea:

One OTHER idea which I want to put on the table for us to think about is 'rewriting history'. Theoretically if a repository doesn't have a consistent interface (throughout time), I'm thinking we could go back in time to add the additional interfaces we want. For example, if build.sh didn't have a --bindir tag, we could go back and ADD it (no removing, no changing what is there already, just ADDING - to escape conflict, etc.). And it'd essentially just be a property setting we'd pipe through to MSBuild or whatever. Any thoughts on this approach?

Obviously this is scary, and rife with political conflict, but technically feasible imo.

Update: Matt couldn't recommend no any faster when I talked to him about this. So, nope.

tools/cli-bootstrap/cli.bootstrap.py
+ RoverSettings.LibUVBinDirectory = default_libuv_bin_directory
+
+ platform_info = platform.uname()
+ this_distro_name = str(platform.linux_distribution()[0]).lower()
@mellinoe

mellinoe Nov 2, 2016

Member

What about non-Linux platforms?

@ellismg

ellismg Nov 2, 2016

Owner

I think it's fine to be specific for Linux right now...

@ghost

ghost Nov 2, 2016

Not on mind right now.

ghost changed the title from Moving Rover (cli-bootstrapping tool) from private repository into dotnet/core to [WIP] Moving Rover (cli-bootstrapping tool) from private repository into dotnet/core Nov 3, 2016

ghost added the wip label Nov 3, 2016

ghost changed the title from [WIP] Moving Rover (cli-bootstrapping tool) from private repository into dotnet/core to Moving Rover (cli-bootstrapping tool) from private repository into dotnet/core Nov 3, 2016

ghost changed the title from Moving Rover (cli-bootstrapping tool) from private repository into dotnet/core to Moving Rover (dotnet-bootstrapping tool) from private repository into dotnet/core Nov 16, 2016

bryanar and others added some commits Nov 1, 2016

@bryanar bryanar Moving Rover (cli-bootstrapping tool) from private repository, into p…
…ublic
c0b4aa2
@bryanar bryanar we'd like line endings to be normalized 23c1fca
@bryanar bryanar we do not want to install dependencies for the
end-user
db9ade7
@bryanar bryanar updated documentation 63d7044
@bryanar bryanar These are not necessary b2893c1
@bryanar bryanar only use colors if we are in a real terminal a6f7eac
@bryanar bryanar Laying down docker interfaces
d5ae4db
root cleaning up old stuff
9741287
root testing ci
c95654b
root adjusting working directory
f895ac6
@ghost

ghost commented Nov 21, 2016

test ci please

@ghost

ghost commented Nov 21, 2016

CI test is passing, going to hold off on one more thing - I only want the CI to run when modifying dotnet.bootstrap.py

Member

tmds commented Nov 21, 2016

I tried the dotnet.bootstrap.py tool on Fedora 25.

What I found out:

  • The tool does not take care of dependencies, which is okay because each repo does a pretty good job of reporting it's own dependencies. It means "development mode" is not exceptional but it is a typical part of bootstrapping. Why isn't this the 'only' mode?
  • The script files which are created to debug a failing build step miss the execute flag. They need a manual chmod +x.
  • Each time I restarted the tool, it downloaded the cli again saying "Could not locate a payload at path ''". Perhaps the tool can store the downloaded file as dotnet.tar.gz file and automatically use it if no other payload was provided?
  • I don't get what the -nopatch is for. The goal of the tool is to patch? Or perhaps I don't understand what the nopatch does.
  • I had to make some changes to the repositories which were already fixed on master due to the compiler being a more recent one. This is a normal part of bootstrapping.
  • In the end, I had a dotnet. It managed to 'dotnet new' and 'dotnet restore'. But the 'dotnet build' hang. It was using msbuild. This was a bit of a surprise since there hasn't been an SDK release with msbuild. How does the tool chose a tar.gz file to download? And how does it chose the tags/branches of the different repositories it is using?
  • At that point I decided to manually pick the fedora LTS release and provide it using the -payload option. It took me a bit longer to find on the dot.net website that it would for a Windows/Mac user. When running it providing the payload option I got an uncaught exception: "SemanticVersion instance has no attribute 'find'" of type <type 'exception.AttributeError'>.
  • Not sure what to delete, I deleted the fedora.25-x64-dotnet folder to get things in order again. I didn't expect the src folder to be in there, so this accidentally caused me to delete the changes I made to the repositories. Perhaps the src folder can move up one level?

All in all, I liked how the tool helps bootstrapping. Using the scripts, it has a nice workflow a la "bootstrap --continue".

@ghost

ghost commented Nov 21, 2016

@tmds Fantastic feedback! Thank you!

  • "The tool does not take care of dependencies" - True, this was snipped off (too many instances out there where dependencies can't just be 'guaranteed' which makes the feature useless except for cases that are 'well known', but in the well known instances we are less likely to need this. If really in a bind for dependencies, review the docker containers in base/lab/containers that has the apt-get, yum, zypper, dnf, etc. commands to pull in packages. @gkhanna79 (Relevant to you).

  • "The script files which are created to debug a failing build step miss the execute flag" - Will fix.

  • "It downloaded the CLI again..." - Will fix.

  • "What is no patch?" - This is an experimental feature that probably does not need to exist any longer (i.e. I'll remove it after a bit more review).

  • "How does the tool choose?" - At the moment it does no choosing - we simply grab the latest debian dotnet bits, then we grab specific checkout commits and build, patch, run. In the future we will take the version number/commit hash from the binary and build only those pieces (in other words, whatever binary versions you provide (via -payload; or whatever we download) we will reproduce for the selected platform).

  • Given your story the DevMode makes me think this has caused a headache having this distinction. I too think DevMode should be 'default' now - or at least removed. I also like your idea of 'bootstrap --continue' - probably going to steal that ;)

Again, thank you for sharing your experience with using this tool!

Member

tmds commented Nov 23, 2016 edited

Second attempt of bootstrapping on Fedora 25.

Install Fedora 25

Used iso: https://download.fedoraproject.org/pub/fedora/linux/releases/25/Workstation/x86_64/iso/Fedora-Workstation-Live-x86_64-25-1.3.iso

Install dependencies

$ sudo dnf -y install python cmake clang llvm libicu-devel lldb-devel libunwind-devel lttng-ust-devel libuuid-devel zlib-devel libcurl-devel krb5-devel openssl-devel automake libtool

Get Rover

$ git clone https://github.com/dotnet/core
$ cd core
$ git checkout bootstrap
$ git rev-parse HEAD
f895ac6b595ff03a22bd1278f803dbfe29c61e75
$ cd tools/dotnet-bootstrap

Get payload SDK

Using Fedora 24 .NET Core 1.1 SDK:

$ curl -sSL -o dotnet.tar.gz https://go.microsoft.com/fwlink/?LinkID=835025

Select your branches

$ mkdir -p fedora.25-x64-dotnet/src
$ cd fedora.25-x64-dotnet/src
$ git clone https://github.com/dotnet/corefx
$ cd corefx
$ git checkout release/1.1.0
$ git rev-parse HEAD
639eeadbf43c0b2be334ea0ee83ae9da2ca03578
$ cd ..
$ git clone https://github.com/dotnet/coreclr
$ cd coreclr
$ git checkout release/1.1.0
$ git rev-parse HEAD
673561935b3112e1128392b61b4e974f1b04545c
$ cd ../..

I let rover pick branches for libuv and core-setup:

core-setup at v1.1.0 (928f77c4bc3f49d892459992fb6e1d5542cb5e86)
libuv at v1.9.0 (229b3a4cc150aebd6561e6bd43076eafa7a03756)

Bootstrap

$ ./dotnet.bootstrap.py -payload dotnet.tar.gz
...
** ROVER spawned a 'dotnet' in ./fedora.25-x64-dotnet/bin/(enjoy!)

Does it work?

Edited:
First time I tried this dotnet new;dotnet restore;dotnet run got stuck compiling.
Retried everything from scratch after merge (core at 1f2bab7) with same versions for coreclr, corefx, libuv and core-setup and it was working fine. Unclear what the root cause was for the fail.

Owner

ellismg commented Nov 28, 2016

This PR also adds a ubuntu directory with submodules. Was that expected or was output from a local invocation of the tool accidentally included?

@ghost

ghost commented Nov 28, 2016

@ellismg Accident. I will fix this up and close it up now actually.

@ghost

ghost commented Nov 28, 2016

@tmds Oops, I do actually have the check for chmod +x on the repro scripts. If you run the script in sudo mode this is the only time I will set the X bit on the repro script.

@bryanar bryanar do not keep retrieving the patch target
01b5f4a
@ghost

ghost commented Nov 28, 2016

test ci please

ghost removed the wip label Nov 28, 2016

@ghost ghost merged commit 927a249 into master Nov 29, 2016

3 checks passed

Ubuntu Debug Build finished.
Details
Ubuntu Release Build finished.
Details
dotnet_core/master/GenPRTest/generator_prtest Build finished.
Details

tmds referenced this pull request in dotnet/cli Nov 29, 2016

Closed

Support for libicu 57 (default for Fedora 25) #4865

FYI: moby/moby@4adf74a was merged and fix is available in Docker v1.13.0.

bernardbr referenced this pull request in OmniSharp/omnisharp-vscode May 30, 2017

Open

Cannot debug a C# application #1526

richlander deleted the bootstrap branch Jul 10, 2017

This issue was closed.

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