Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 <byronimo@gmail.com>
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
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

4 participants