Arvados

Perhaps check whether SETUP_DIR/../../.git exists in sdk/python/setup.py before trying to get a version date from git log?

Reviewing 21ead51

There are many things that could theoretically go wrong trying to get the git log. I worry that tossing all of git's stderr into the bit bucket will likely mask the issue if we ever do run into real trouble building the packages.

What if we only ran git when the PKG-INFO file is absent? setuptools automatically generates and includes that file in built source distributions, and there's probably no circumstances we should be trying to override the build tag in that case.

Brett Smith wrote:

Reviewing 21ead51

There are many things that could theoretically go wrong trying to get the git log. I worry that tossing all of git's stderr into the bit bucket will likely mask the issue if we ever do run into real trouble building the packages.

My approach here was really just to take the story at face value: leave the existing logic of "we don't care if this fails" alone and acknowledge that in that case we don't care why it fails.

What if we only ran git when the PKG-INFO file is absent? setuptools automatically generates and includes that file in built source distributions, and there's probably no circumstances we should be trying to override the build tag in that case.

Yes, that sounds better, I was hoping you (or someone) would point out a way of knowing whether we're in "build" or "install". I would much rather have this:

• if we need a git hash
    run git
    if that fails
      show error message and abort
  

...than this:

• run git
  if that fails
    we probably don't need a git hash, so set git_tags=None and hope for the best
  

Will update accordingly.

Tom Clegg wrote:

Brett Smith wrote:

What if we only ran git when the PKG-INFO file is absent? setuptools automatically generates and includes that file in built source distributions, and there's probably no circumstances we should be trying to override the build tag in that case.

Yes, that sounds better, I was hoping you (or someone) would point out a way of knowing whether we're in "build" or "install".

Well, I didn't really do that. Looking for PKG-INFO just a cheap heuristic that I invented by comparing my git checkout with a source build. If you combine that with hard-failing on git errors, you'll break a few hypothetical use cases. For example, someone who gets a source distribution, patches it, and wants to make a new source distribution to use internally will trip when setup.py calls git. Maybe we don't want to support that; just noting it. [Edit: Well, okay, maybe not depending on whether and when PKG-INFO sticks around, but the point remains that the existence of the file doesn't authoritatively answer the question either way.]

The super proper way to make everybody happy here would be to extend the egg_info command in the Setuptools classes, so that if it gets called without any build tag available, it tries to generate one from git and raises an error if it can't. Source distributions will include the build tag in setup.cfg, which will be passed into the egg_info command, so anybody working from that will be covered; and I think we're willing to assume that anybody else must be working from git. I started going down this road when I implemented the current git versioning code, and ditched it when I found the options argument during that work. But if we want to be this context-sensitive, I think it's reasonable that we'd want to hook into lower levels of the system.

Cleaner solution at 992ddfa on 4400-python-install-herring.

If there is no build tag, use git commit date+hash; if anything fails, you get git's stderr (if any) and a Python exception.

If there is a build tag (e.g., you're installing a package that's already built, or "egg_info -b" happened), don't touch the build tag, don't run git.

Tom Clegg wrote:

Cleaner solution at 992ddfa on 4400-python-install-herring.

Two small things from here:

• Please apply the same patch to Node Manager's setup.py
• Two lines after a class definition would be nicely PEP 8 compliant.

Please go ahead and merge after that. Thanks.

• Build and install a nodemanager package
• Dry up python package test/install recipes
• python_sdk_test=FOO is now sdk/python_test=FOO

5ab76d5 looks great. I tested it with different combinations of --skip, --only, and sdk/python_test=, and everything works nicely. Thanks.