Skip to content

Pr cmd raise with stderr on error #452

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

Merged
merged 7 commits into from
Jun 13, 2016
Merged

Pr cmd raise with stderr on error #452

merged 7 commits into from
Jun 13, 2016

Conversation

barry-scott
Copy link
Contributor

I have seen the stderr for both pull() and push() in GitCommandError with this patch.

@Byron
Copy link
Member

Byron commented May 30, 2016

Hey Barry,

Thanks a bunch for your contribution ! Could you also have a look at travis please ? It seems that a few tests are failing. They probably need to be adjusted or something did break indeed.

You will find everything you need to run the tests in the .travis.yml file in the root of this project, alternatively you should be able to run nosetests git/test, which is what I do quite a lot. flake8 is easily forgotten here, but can also cause travis to not green-light the PR.

Thanks

for line in proc.stderr:
line = force_text(line)
for pline in progress_handler(line):
if line.startswith('fatal:') or line.startswith('error:'):
raise GitCommandError(("Error when fetching: %s" % line,), 2)
error_message = "Error when fetching: %s" % (line,)
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it' s enough to break after the first line indicating a problem ? My intuition is that the lines that follow on stderr contain context, that you probably want to catch as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works because of the 2nd break at line 591.

Also I see this work in testing.

@barry-scott
Copy link
Contributor Author

Found flake8 in pip and used to to fix the complaints.
python3 -m pip install flake8

Took a moment to figure out where nosetests came from.
dnf install python3-nose

I see errors from running the tests I'll work on there a bit later. may be tomorrow before I can push a update.

remove stderr for a wait() that is not the GitPython wrapper.
@Byron Byron added this to the v2.0.6 - Bugfixes milestone Jun 1, 2016
@Byron
Copy link
Member

Byron commented Jun 1, 2016

Good to hear you can now test locally !
Please note that it's advised to run only the tests you are interested in (e.g. nosetests git/test/test_remote.py, as some could fail if they are run on a branch which is not named master.
One good way to workaround is to just work on master.
Next thing to do is to execute the commands that you find in the .travis.yml file to assure the pre-requisites are met. Reason for this requirement is that the tests actually use their own repository (which violates sandboxing).

With all that in place, you should reach a point where all tests run successfully on your own machine, in case everything works.

@barry-scott
Copy link
Contributor Author

I have instrumented to try and find out what is happening in one of the test failures.

What I see is this command that fails with 128.

git clone --progress --separate-git-dir=/home/barry/tmpdir/test_renamescm7sp1x/parent/.git/modules/mymodules/myname -v /home/barry/wc/git/GitPython/git/ext/gitdb/gitdb/ext/smmap /home/barry/tmpdir/test_renamescm7sp1x/parent/mymodules/myname

Notice the /home/barry/wc/git/GitPython/git/ext/gitdb/gitdb/ext/smmap path.
I have /home/barry/wc/git/GitPython/git/ext/gitdb as am empty directory.
Where does the other files come from in that folder?

@barry-scott
Copy link
Contributor Author

Sorry I missed your reply about testing.

Work on master?! But isn't that a git sin?

Are you saying that I need to run all the commands in the install: section?

@barry-scott
Copy link
Contributor Author

barry-scott commented Jun 6, 2016

This failure is becuase the GIT_SSH is not making it in to Popen call.
There is odd code that seems to work around a expected failed that I do not understand in the test as well. Taken from tst_git.py around line 205:

        with rw_repo.git.custom_environment(GIT_SSH=path):
            try:
                remote.fetch()
            except GitCommandError as err:
                if sys.version_info[0] < 3 and sys.platform == 'darwin':
                    assert 'ssh-origin' in str(err)
                    assert err.status == 128
                else:
                    assert 'FOO' in str(err), 'err is %r' % (err,)
                    assert err.status == 2

Why expect FOO to be missing an python 2 and darwin?
Its missing with python3 and linux for me.

$ nosetests-3.4 git/test/test_git.py 
......Test TestGit.test_environment failed, output is at '/home/barry/tmpdir/test_environment74vf1sim'
F...........
======================================================================
FAIL: test_environment (git.test.test_git.TestGit)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/barry/wc/git/GitPython/git/ext/gitdb/gitdb/test/lib.py", line 87, in wrapper
    return func(self, path)
  File "/home/barry/wc/git/GitPython/git/test/test_git.py", line 213, in test_environment
    assert 'FOO' in str(err), 'err is %r' % (err,)
nose.proxy.AssertionError: err is GitCommandError(['git', 'fetch', '-v', 'ssh-origin'], 128, b'')
-------------------- >> begin captured stdout << ---------------------
failing_script is '/home/barry/tmpdir/test_environment74vf1sim/failing-script.sh'

--------------------- >> end captured stdout << ----------------------

-------------------- >> begin captured logging << --------------------
git.cmd: INFO: env GIT_SSH=None
git.cmd: INFO: env GIT_SSH=None
git.cmd: INFO: env GIT_SSH=None
git.cmd: DEBUG: AutoInterrupt wait stderr: b''
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 18 tests in 0.551s

FAILED (failures=1)

@barry-scott
Copy link
Contributor Author

Can you try this and see if the tests pass for you?

I fixed the test that looked fro FOO, test case error.

Given I'm not on master I see that test fail as well.

I am seeing some odd errors from test_repo, but suspect that I have triggered a bug somewhere else.
My .git/logs/refs/heads/pr-cmd-raise-with-stderr-on-error may be corrupt; I commited using GitPython.

@Byron Byron merged commit 6891caf into gitpython-developers:master Jun 13, 2016
@Byron
Copy link
Member

Byron commented Jun 13, 2016

Please see if my merge is still working. If not, please add a test that fails unless the fix you provide is applied.
Thanks for your contribution.

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

Successfully merging this pull request may close these issues.

2 participants