GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — FIx issue #464 corrupt log file by barry-scott · Pull Request #469 · gitpython-developers/GitPython

Conversation

@barry-scott

@barry-scott barry-scott commented Jun 13, 2016

Copy link
Copy Markdown
Contributor

Must only append the first line of the commit to the log
Otherwise the log file is not parsable by GitPython.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
barry-scott added 2 commits June 13, 2016 15:26
@barry-scott
Fix corruption of the ref logs file
e0eafc4 
It must only have the first line of the
commit messages, not the while multiple line log.
@barry-scott
fix flake8 found problems
a7f403b
@Byron Byron added the waiting for feedback label Jun 14, 2016
@Byron Byron added this to the v2.0.6 - Bugfixes milestone Jun 14, 2016
@Byron

Byron commented Jun 14, 2016

Copy link
Copy Markdown
Member

Do you think it's possible for you to write a test which goes red if the change is not there ? The current test-suite should be a good starting-point for a fixture-based test.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Byron Byron mentioned this pull request Jun 14, 2016
GitPython Commit corrupts .git/logs/refs/heads/pr-cmd-raise-with-stderr-on-error #464
Closed
@barry-scott

barry-scott commented Jun 14, 2016

Copy link
Copy Markdown
Contributor Author

Please don't wait for me to write the test. I have a lot of work on my plate at moment.
Maybe merge this and leave the bug open awiting a test case?

How I detected the bug was:

  1. Use GitPython to commit to a repo - causing the fault
  2. Use GitPython to scan the log ref file - detecting the fault
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Byron Byron added help wanted and removed waiting for feedback labels Jun 18, 2016
@nvie nvie modified the milestones: v2.0.6 - Bugfixes, v2.0.7 - Bugfixes Jun 20, 2016
@barry-scott
Merge branch 'master' of https://github.com/gitpython-developers/GitP…
b4b5ecc 
…ython
@Byron Byron modified the milestones: v2.0.7 - Bugfixes, v2.0.8 - Bugfixes Jul 17, 2016
barry-scott added 2 commits July 29, 2016 11:33
@barry-scott
Merge remote-tracking branch 'upstream/master'
1116ef7
@barry-scott
Prevent CMD windows being shown when starting git in a subprocess.
0d93908 
This fixes a UI problem with using GitPython from a GUI python probgram.
Each repo that is opened creates a git cat-file processs and that provess will create
a console window with out this change.
@barry-scott

barry-scott commented Jul 29, 2016

Copy link
Copy Markdown
Contributor Author

I'm not sure how to write the tests you want.

Can you point to docs on what is required?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Byron
Byron reviewed Jul 30, 2016
View reviewed changes
Comment thread git/cmd.py Outdated
else:
creationflags = None

p = Popen(['ps', '--ppid', str(pid)], stdout=PIPE, creationflags)

@Byron Byron Jul 30, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am afraid that using creationflags as positional argument doesn't actually end up in the correct spot. Also I thought that positional arguments after keyword args are not allowed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Byron Byron Jul 30, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This concern is reflected in actual breakage as well: https://travis-ci.org/gitpython-developers/GitPython/jobs/148304657#L318

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Byron

Byron commented Jul 30, 2016

Copy link
Copy Markdown
Member

Now this PR contains two distinct and independent changes. The most recent one is hard to test, and I'd be fine without one.
As for the first set of changes, I believe I already referred to the respective test-suite. Generally, you want to write a test that fails until your changes are applied. This is the whole trick. The suite in question should already contain everything you need to setup your own.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Byron Byron mentioned this pull request Jul 30, 2016
CMD windows created for Windows GUI python programs #495
Closed
@barry-scott
Must pass creationflags as a keywork
d79951f
@barry-scott

barry-scott commented Aug 1, 2016

Copy link
Copy Markdown
Contributor Author

I fixed the keyword parameter passing.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
barry-scott added 2 commits August 1, 2016 11:09
@barry-scott
creationflags must be set to 0 on non-windows platforms
572ebd6
@barry-scott
add test to detect the corrupt log
8bde103 
- add a second line to commit messages with the "BAD MESSAGE" text
- read in the log and confirm that the seond line is not in the log file
@barry-scott

barry-scott commented Aug 1, 2016

Copy link
Copy Markdown
Contributor Author

I have added a basic test by changing one of the existing test_repo.py tests.

You can see the test fail by undoing the first line fix and running the test.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barry-scott
fix flake8 problems
d8ef023
@Byron
Byron merged commit d8ef023 into gitpython-developers:master Aug 2, 2016
@Byron

Byron commented Aug 2, 2016

Copy link
Copy Markdown
Member

@barry-scott Thank you, looks very good now ! I am particularly excited about the test, and hope in future you can give test-first development a shot as well.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barry-scott

barry-scott commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

To get the test as one patch and the the test + fix as a second you will have to fix the dependency of the tests on master. Use of master means that you do not have a reproducible test. Its inputs depend on what the user did to create there master.

I suspect that if you had to create the test data that you use master for you might have found this bugs ages ago.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@EliahKagan EliahKagan mentioned this pull request Dec 22, 2023
USE_SHELL = True not needed since 2.0.8 but still suggested for Windows #1781
Closed
@EliahKagan EliahKagan mentioned this pull request Jan 16, 2024
Fix two remaining Windows untrusted search path cases #1792
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

help wanted

Milestone

v2.0.8 - Bugfixes

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@barry-scott @Byron @nvie
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.