Bug #4400

[SDKs] Python package install should not print misleading message "fatal: Not a git repository"

Added by Peter Amstutz about 6 years ago. Updated about 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Target version:
Start date:
11/18/2014
Due date:
% Done:

100%

Estimated time:
(Total: 0.50 h)
Story points:
0.5

Description

Installing the arvados_fuse and arvados-python-client packages produces the following output:

Unpacking ./dist/arvados_fuse-0.1.20141017184025.d901428.tar.gz
  Running setup.py (path:/tmp/pip-hgXXvG-build/setup.py) egg_info for package from file:///home/peter/work/arvados/services/fuse/dist/arvados_fuse-0.1.20141017184025.d901428.tar.gz
    fatal: Not a git repository (or any of the parent directories): .git

  Requirement already satisfied (use --upgrade to upgrade): arvados-fuse==0.1.20141017184025.d901428 from file:///home/peter/work/arvados/services/fuse/dist/arvados_fuse-0.1.20141017184025.d901428.tar.gz in /home/peter/work/arvados/services/fuse
Downloading/unpacking arvados-python-client>=0.1.1411069908.8ba7f94 (from arvados-fuse==0.1.20141017184025.d901428)
  Downloading arvados-python-client-0.1.20141024175815.ef56ac5.tar.gz (71kB): 71kB downloaded
  Running setup.py (path:/tmp/tmp.4LayB3OknI/build/arvados-python-client/setup.py) egg_info for package arvados-python-client
    fatal: Not a git repository (or any of the parent directories): .git

The text "fatal: Not a git repository (or any of the parent directories): .git" is not actually a fatal error and should be fixed or suppressed.


Subtasks

Task #4636: Review 4400-dry-python-tests on arvados-devResolvedTom Clegg

Task #4576: Review 4400-python-install-herringResolvedTom Clegg

Associated revisions

Revision ba251e27
Added by Tom Clegg about 6 years ago

Merge branch '4400-python-install-herring' refs #4400

Revision 8a2221be
Added by Tom Clegg about 6 years ago

Merge branch '4400-dry-python-tests' refs #4400

Revision 8a2221be
Added by Tom Clegg about 6 years ago

Merge branch '4400-dry-python-tests' refs #4400

Revision e52b761b (diff)
Added by Tom Clegg about 6 years ago

Ignore failure at "easy_install *.egg". refs #4400

Revision e52b761b (diff)
Added by Tom Clegg about 6 years ago

Ignore failure at "easy_install *.egg". refs #4400

History

#1 Updated by Peter Amstutz about 6 years ago

  • Description updated (diff)

#2 Updated by Peter Amstutz about 6 years ago

  • Subject changed from Python package install prints misleading message " fatal: Not a git repository" to Python package install prints misleading message "fatal: Not a git repository"

#3 Updated by Tom Clegg about 6 years ago

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

#4 Updated by Ward Vandewege about 6 years ago

  • Target version changed from Bug Triage to Arvados Future Sprints

#5 Updated by Tom Clegg about 6 years ago

  • Subject changed from Python package install prints misleading message "fatal: Not a git repository" to [SDKs] Python package install should not print misleading message "fatal: Not a git repository"
  • Story points set to 0.5

#6 Updated by Tom Clegg about 6 years ago

  • Status changed from New to In Progress

#7 Updated by Tom Clegg about 6 years ago

  • Target version changed from Arvados Future Sprints to 2014-12-10 sprint

#8 Updated by Brett Smith about 6 years ago

  • Assigned To set to Tom Clegg

#9 Updated by Brett Smith about 6 years ago

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.

#10 Updated by Tom Clegg about 6 years ago

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.

#11 Updated by Brett Smith about 6 years ago

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.

#12 Updated by Tom Clegg about 6 years ago

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.

#13 Updated by Brett Smith about 6 years ago

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.

#14 Updated by Tom Clegg about 6 years ago

4400-dry-python-tests at arvados-dev|5ab76d5
  • Build and install a nodemanager package
  • Dry up python package test/install recipes
  • python_sdk_test=FOO is now sdk/python_test=FOO

#15 Updated by Brett Smith about 6 years ago

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

#16 Updated by Tom Clegg about 6 years ago

  • Category set to SDKs
  • Status changed from In Progress to Resolved

Also available in: Atom PDF