Handle bytes when using gitpython with python3.x

[vin01]

What does this PR do?

Use BytesIO fake file object instead of StringIO to support gitpython with Python3.x

What issues does this PR fix or reference?

#54402

Tests written?

Fixed existing test

Commits signed with GPG?

Yes

Note: Please see #54453, #54454 for individual Jenkins CI results for these changes. Because of merge conflicts and base branch change, I am re-submitting these changes here.

[waynew]

Do we have any tests that shows the bytes/text piece failing? I haven't seen any failing tests on master around this code. 🤔

Also, are there any supported versions of gitpython that behave differently? What about pygit2?

Just want to make sure that this doesn't cause any breakages in the other direction 😝

[vin01]

Thanks for reviewing @waynew

Regarding failing tests, tests were not failing because the case where symlinks are handled (https://github.com/saltstack/salt/pull/54900/files#diff-2c34a66e40db8ce28e251f98de56ce31R1309) was never triggered because symlinks from the test repo during test-run were not being copied as symlinks but regular files.

I also validated this on the jenkins CI job for Salt but looks like now the build is deleted most likely because of retention kick-in. (https://jenkinsci.saltstack.com/job/pr-kitchen-debian9-py3/job/PR-54453/1/console)

This is only applicable to gitpython (all versions) and does not affect pygit2. This code block is specific to gitpython. I hope that helps.

[dwoz]

@vin01 Please address the merge conflicts

[vin01]

@vin01 Please address the merge conflicts

Resolved. Failed builds seem unrelated to me from the Jenkins console output.

it will be nice to have this finally merged some time soon.

[vin01]

It has been quite a while now for this rather small fix to get through.

I will appreciate a decision on this soon as I am still hooked to a fork version and I was really not expecting it to take this long.

tl;dr we need to copy symlinks as symlinks to be able to test all scenarios which the tests are supposed to cover. I do not see absolutely any reason for it to be otherwise. am I missing something?

[waynew]

Only two builds need to pass now - one failed but I'm pretty sure that was due to infrastructure issues. (py2/amazon1). I've restarted that build.

[vin01]

Looks like the build (py2/amazon1) exceeded the timeout so got Aborted.

03:11:43  Cancelling nested steps due to timeout
03:12:53  Setting currentBuild.result to ABORTED
03:12:55  Timeout has been exceeded
03:12:55  Finished: ABORTED

Second build has been removed so can't really check.

[dwoz]

@vin01 Thanks for your patience and working with us to get this merged. We're making progress on the backlog of PRs recently so hopefully next time your changes will go through with much less friction.