Skip to content

TST: test on travis while TMPDIR containing unicode chars #543

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

Closed

Conversation

yarikoptic
Copy link
Contributor

by popular demand ;)

ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 23, 2016
Try to fix gitpython-developers#543 unicode woes in "core" position (ie TMPDIR):
+ Apply surrogate-escapes(PEP383) also when decoding.
+ Ensure all file-path and cmd-streams are surogate-escape dencoded.
+ test_utils: check if lock works with unicodes.
+ git.compat: FIX undefined exc to raise in `replace_surrogate_encode()`
and fkale8 fixes.

ci results:
 + Linux:
   + py2.7 FAIL 53 TCs
   + py3: fixed
 + Windows, all were OK
@ankostis
Copy link
Contributor

ankostis commented Oct 23, 2016

I managed to fix PY3 TCs with f801a3a, but unfortunately, PY2 encodings for filepaths is a "casino"; surprisingly, Windows had no problem! Cannot reproduce this under Windows, win-git refused to start with unicode TMPDIR vars.

@@yarikoptic can you live by that alone?

ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/gitdb that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/gitdb that referenced this pull request Oct 23, 2016
@ankostis
Copy link
Contributor

ankostis commented Oct 23, 2016

Both smmap and gitdb projects fails PY2 with a unicode tmp-dir :-(~~
[edit:] False alarm for smmap; PY2 was failing when pip-installing codecov; pip is not happy with symlinked unicode temp-folders...

ankostis added a commit to ankostis/smmap that referenced this pull request Oct 23, 2016
ankostis added a commit to ankostis/gitdb that referenced this pull request Oct 23, 2016
@yarikoptic
Copy link
Contributor Author

Dear @ankostis - if you are still interested in my opinion/desires -- I just want ideally both PY2 and PY3 supported. ATM we do not actively support Windows (do Linux and OSX), so those platforms are of interest to me

@ankostis
Copy link
Contributor

ankostis commented Oct 27, 2016

Of course!
Why do you say so?
[edit:] I run TCs also on Linux. But I cannot fix it.
My comment about Windows, was for-the-record, nothing to do with my efforts to fix Linux-PY2.

@yarikoptic
Copy link
Contributor Author

Saying just because you asked and I didn't reply ? In time so thought that may be you fixed it all and no feedback was needed any longer ;-)

@ankostis
Copy link
Contributor

Anyhow, I've managed to fix PY3 on linux, please do try and fix the issue also in PY2; commence from f801a3a, as explained above.

@yarikoptic
Copy link
Contributor Author

I ahead stashed my laptop away for a transatlantic flight to commence in few minutes... Will try when across the pond and finish driving 300 miles north unless someone beats me to it ;-)

ankostis added a commit to ankostis/smmap that referenced this pull request Oct 28, 2016
ankostis added a commit to ankostis/smmap that referenced this pull request Oct 28, 2016
@Byron
Copy link
Member

Byron commented Dec 8, 2016

@yarikoptic I do hope you have landed safely :)! I would also be looking forward to your results whenever you have some time.

@yarikoptic
Copy link
Contributor Author

thanks for the buzz @Byron ! Yes, I have landed successfully ;) and today seems to be the day to respond to my own whinings on github (trying to fixup pandas as well atm).

Could you please clarify where that commit/branch you pointed to lives at since I can't find it on a quick look :-/

$> git log | grep surrogate
    fix(surrogateescape): enable on py2, fix tests
    fix(unicode): use surrogateescape in bytes.decode
changes on filesystem:                                                           
 git/ext/gitdb | 2 +-

$> git show f801a3aed7ab28b1c80fc1b573afb2b182eca995
fatal: bad object f801a3aed7ab28b1c80fc1b573afb2b182eca995

$> git show | head
commit b0c187229cea1eb3f395e7e71f636b97982205ed
Author: Sebastian Thiel <[email protected]>
Date:   Thu Dec 8 16:07:11 2016 +0100

meanwhile I will merge current master, resolve conflict and push...

* upstream/master:
  chore(lint): flake8 pacification
  fix(refs): handle quoted branch names
  chore(repo): remove comment
  chore(lint): flake8
  fix(submodule): don't fail if tracking branch can't be setup
  Don't change the meaning of string literals
  Fixes to support Python 2.6 again.
@codecov-io
Copy link

codecov-io commented Dec 8, 2016

Codecov Report

Merging #543 into master will decrease coverage by 8.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #543      +/-   ##
==========================================
- Coverage   94.66%   86.36%   -8.31%     
==========================================
  Files          60       60              
  Lines        9340     9338       -2     
==========================================
- Hits         8842     8065     -777     
- Misses        498     1273     +775
Impacted Files Coverage Δ
git/test/test_tree.py 18.33% <0%> (-81.67%) ⬇️
git/test/test_remote.py 31.33% <0%> (-66.49%) ⬇️
git/test/test_submodule.py 52.76% <0%> (-46.48%) ⬇️
git/odict.py 20% <0%> (-30%) ⬇️
git/objects/submodule/root.py 64.39% <0%> (-28.04%) ⬇️
git/compat.py 40.74% <0%> (-24.29%) ⬇️
git/index/fun.py 88.82% <0%> (-8.83%) ⬇️
git/test/test_base.py 90.8% <0%> (-8.05%) ⬇️
git/test/test_index.py 88.53% <0%> (-7.94%) ⬇️
git/test/lib/asserts.py 61.53% <0%> (-7.7%) ⬇️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f14a596...aace290. Read the comment docs.

@Byron
Copy link
Member

Byron commented Dec 9, 2016

@yarikoptic Actually I don't know where said commit is, maybe @ankostis can help finding it. So far this issue seemed like a conversation between you to, I just wanted to reignite it after the transatlantic flight.

@ankostis
Copy link
Contributor

ankostis commented Dec 9, 2016

@Byron
Copy link
Member

Byron commented Mar 8, 2017

Is there something we should do with this PR?

@ankostis
Copy link
Contributor

ankostis commented Mar 8, 2017

I've managed to address the problem described only for python-3, as explained in this comment.

@Byron
Copy link
Member

Byron commented Apr 9, 2017

@ankostis Thanks again for your contribution, and sorry for the great delay (once again). It appears it is still having trouble to deal with this :/.

* upstream/master: (83 commits)
  Remove trailing slash on drive path
  Further update for machines without ssh installed or on the path
  Update remote.py to fix issue gitpython-developers#694
  IndexFile.commit() now runs pre-commit and post-commit and commit-msg hooks.
  Update base.py
  Update remote.py
  Update base.py
  Update remote.py
  Update signing key to latest version
  recognize the new packed-ref header format
  Only gc.collect() under windows
  Converting path in clone and clone_from to str before any other operation in case eg pathlib.Path is passed
  Fix encoding issue with stderr_value and kill_after_timeout
  Store submodule name
  updating AUTHORS
  Keeping env values passed to `clone_from`
  Fix test_docs
  Apparently bdist_wheel is only in python3
  version bump
  BF: Added missing NullHandler to logger in git.remote
  ...
@yarikoptic
Copy link
Contributor Author

pushed with current master merged in -- may be some things are already better ;-)

@Byron
Copy link
Member

Byron commented Dec 11, 2017

@yarikoptic This one will probably have to wait for another release. The current one should already make things better though :).

@yarikoptic
Copy link
Contributor Author

sure! I will merge current improved master again to see where we stand though
Cheers

* upstream/master:
  Add Yarikoptic to allowed release keys
  Remove redundant Python 2.4 code
  Specify Python 3.6 support
  to keep travis busy - adding myself to AUTHORS
  BF(WIN): where  could report multiple hits, so choose first
  RF: use HIDE_WINDOWS_KNOWN_ERRORS instead of is_win to skip hooks tests
  BF(WIN): use  where instead of which  while looking for git
  RF(TST): skip all tests dealing with hooks on windows
  Disable (but keep for future uses commented out) hook into appveyor session
  RF: no "need" for custom shebang on windows since just does not work
  ENH: also report where on sh, and echo msg when entering on_finish
  ENH: add appveyor recipe to establish rdesktop login into the test box
  RF(+BF?): refactor hooks creation in a test, and may be make it compat with windows
  RF: last of flake8 fails - avoid using temp variable in a test
  BF: crazy tests ppl pass an object for status... uff -- catch TypeError too
  BF(PY26): {} -> {0}, i.e. explicit index for .format()
  RF: primarily flake8 lints + minor RF to reduce duplication in PATHEXT
  BF: wrap map into list, since iterator is not well digested by GitConfigParser
  BF: process included files before the rest
  version up
@Byron Byron closed this Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants