Making GN bootstrap work from tarball as opposed to git checkout

[Paweł Hajdan, Jr.]

I recently had a GN bootstrap fail this way when executed from extracted tarball as opposed to git checkout:

FAILED: python ../../tools/gn/last_commit_position.py ../../ gen/tools/gn/last_commit_position.h TOOLS_GN_LAST_COMMIT_POSITION_H_

Could not get last commit position.

I'm just wondering, would there be some way to make this optional or non-fatal? One way I can see could be last_commit_position.py outputting "UNKNOWN" instead of failing.

I can totally write patches for this, just wanted to discuss first.

For reference, this seems to be related to the following code in src/tools/gn/gn_main.cc :

// Only the GN-generated build makes this header for now.

// TODO(brettw) consider adding this if we need it in GYP.

#if defined(GN_BUILD)

#include "tools/gn/last_commit_position.h"

#else

#define LAST_COMMIT_POSITION "UNKNOWN"

#endif

and src/tools/gn/BUILD.gn:

action("last_commit_position") {

  script = "last_commit_position.py"

  # This dependency forces a re-run when the code is synced.

  inputs = [

    "//build/util/LASTCHANGE",

  ]

  outfile = "$target_gen_dir/last_commit_position.h"

  outputs = [

    outfile,

  ]

  args = [

    rebase_path("//", root_build_dir),

    rebase_path(outfile, root_build_dir),

    "TOOLS_GN_LAST_COMMIT_POSITION_H_",

  ]

}

# Note for Windows debugging: GN is super-multithreaded and uses a lot of STL.

# Iterator debugging on Windows does locking for every access, which ends up

# slowing down debug runtime from 0:36 to 9:40. If you want to run debug builds

# of GN over the large Chrome build, you will want to set the arg:

#   enable_iterator_debugging = false

executable("gn") {

  sources = [

    "gn_main.cc",

  ]

  deps = [

    ":gn_lib",

    ":last_commit_position",

    "//base",

    "//build/config/sanitizers:deps",

  ]

}

Paweł

[Peter Mayo]

I would be a fan of replacing the "Last commit" with a hash of the source code.

If you need to know the version, you probably want it to be accurate, even amongst local changes.

Keeping a map from has of source code to git hash might also be useful for determining what version someone is using.  It has the advantageous property that how you get the source is actually irrelevant.

Git, tar, SVN, RCS, smoke signals or inspired guesses.

[Brett Wilson]

If this is used only for bootstrapping, I would just undefine GN_BUILD for the bootstrap build. This will make last_commit_position.h not be included (because I didn't hook that up in the GYP build) and the version will be reported as UNKNOWN.

If you're intending people might actually use these builds, I think it would be worthwhile to grab the right revision.

I think it's important that the version number be an increasing integer for checking how old your version is (the reason we added these to git in the first place).

Brett

[Paweł Hajdan, Jr.]

Currently GN_BUILD is always defined in a GN build.

What would be your recommendation for changing this?

Paweł

[Brett Wilson]

Oh, I see now. I was thinking you were talking about the GN bootstrap code which uses a Makefile. This now seems like poor advice.

What do we do in other places for the build/util last change generated headers?

Brett

[Thiago Farina]

On Tue, Nov 17, 2015 at 3:55 PM, Brett Wilson <bre...@chromium.org> wrote:

Oh, I see now. I was thinking you were talking about the GN bootstrap code which uses a Makefile.

Isn't the bootstrap done with Python script that generates the ninja build file? I don't think we use Makefiles there, unless we are talking about something totally different than tools/gn/bootstrap :)

-- 

Thiago Farina

[Paweł Hajdan, Jr.]

In other places we generate e.g. a LASTCHANGE file either in DEPS (developer workflows and bots) or in the tarball generation script. This can work because the file is put inside source repo, not build output directory.

See the following for more details:

https://code.google.com/p/chromium/codesearch#search/&q=lastchange&sq=package:chromium&type=cs

https://code.google.com/p/chromium/codesearch#chromium/src/build/util/lastchange.py&q=lastchange&sq=package:chromium&ct=rc&cd=1&dr=C

https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/recipe_modules/chromium/resources/export_tarball.py&q=export_tarball.py&sq=package:chromium&type=cs&l=14

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/version.gni&q=lastchange&sq=package:chromium&dr=C&l=14

https://code.google.com/p/chromium/codesearch#chromium/src/build/util/BUILD.gn&sq=package:chromium&type=cs

Paweł

[Brett Wilson]

Maybe we just need a LASTCRCOMMIT file too? I feel strongly the GN versions should be incrementing integers rather than hashes.

Brett

[Thiago Farina]

On Fri, Nov 20, 2015 at 3:31 PM, Brett Wilson <bre...@chromium.org> wrote:

Maybe we just need a LASTCRCOMMIT file too? I feel strongly the GN versions should be incrementing integers rather than hashes.

Can we use http://semver.org/?

-- 

Thiago Farina

[Dirk Pranke]

I don't think we should have versions different from the versions in chromium;

so, if we wanted to use something like a semantic version we'd just use the 

chromium version number.

I think using the Cr-Commit-Position is much clearer and easier for the 

moment, though. If we get to a point where we're doing releases of GN

for a broader audience as an independent product, I think it might make

sense to either switch to the matching chromium version or do something else.

-- Dirk

[tomas....@gmail.com]

Hi,

was there any consensus about how to resolve this issue? I hit it when moving Fedora (RHEL) packages to GN from GYP.

Tom

Dne čtvrtek 12. listopadu 2015 13:38:39 UTC+1 Paweł Hajdan Jr. napsal(a):

[Paweł Hajdan, Jr.]

Gentoo is applying this patch: https://gitweb.gentoo.org/repo/gentoo.git/tree/www-client/chromium/files/chromium-last-commit-position-r0.patch?id=4b12cd6638bc61a9b88ab799d4bd7fd99429fe2e

It'd indeed be helpful though if we could make changes in GN code not to require it.

Paweł

[Paweł Hajdan, Jr.]

Friendly ping.

I'd like to better understand the current context. What is "gn --version" (which is what needs LAST_COMMIT_POSITION) used for?

I wonder if src/build/util/LASTCHANGE could be used instead (example contents: LASTCHANGE=c65d0c0a618449f58c3824992f14c458cd2b5830-refs/heads/master@{#403858}). Note that it also uses commit position.

Paweł

[Brett Wilson]

gn --version is used when somebody complains about a bug I can tell them if their version is out-of-date or not. So the important thing to me is that it has the commit position. 

When I wrote this I think LASTCHANGE was only the hash. If this makes things easier, I'm OK if you want to change it.

Brett

[Dirk Pranke]

I think we should try to keep things to just the commit position, but I'm fine w/ getting that from LASTCHANGE instead.

-- Dirk

[Brett Wilson]

I would also slightly prefer just the commit position. I realize it would actually be trivial to parse out of LASTCHANGE and all of the code to call git could be deleted with no visible changes to the user.

Brett