fix issue#577
I'm not sure if there is some other reasons to invoke str.encode('utf-8') here, So adding a wrapper to make sure only str be printed here
str.encode('utf-8')
str
:thumbsup:
Why not just use fixEncoding? Your fixPrint seems to do essentially the same thing.
I not sure this updated comment is any more correct.
for Python3, fixEncoding will return bytes which will be directly printed as b'\xe4\xbd\xa0\xe5\xa5\xbd' fixPrint is supposed to make sure the bytes decoded to '你好' to fix above issue. I think a better choice should be to make sure fixEncoding to always return str, but I'm not sure if it will introduce other problems.
fixEncoding
bytes
fixPrint
wrote a candidate modifying fixEncoding and fixEncodingRecurse here: https://pagure.io/fork/julian8628/koji/c/3d55e9d
fixEncodingRecurse
~~I cannot replicate #577 with client HEAD (or from koji-1.13.0 release). Can you?~~
Nevermind, got it
Interestingly, fixEncoding is barely used client side. It's used several places in the hub, web, and builder, but in the client lib the only place is in parse_pom (which itself is mostly used on hub/builder, but is used in the import-archive command).
parse_pom
Point being, there's not a lot of places where fixEncoding runs under python3 right now, pretty much just import-archive --type maven.
import-archive --type maven
So I wonder if import-archive works on py3 as-is, or if it would work better if fixEncoding returned str.
import-archive
btw, the .encode('utf-8') was added here:
commit e565217a73e82ee1f58c7f170bb8bdec76c66e99 Author: Mike McLean <mikem@redhat.com> Date: Tue Dec 8 22:40:05 2009 -0500 avoid unicode errors involving changelogs (rhbz 545387, patch by dmach)
https://bugzilla.redhat.com/show_bug.cgi?id=545387
Interestingly, fixEncoding is barely used client side. It's used several places in the hub, web, and builder, but in the client lib the only place is in parse_pom (which itself is mostly used on hub/builder, but is used in the import-archive command). Point being, there's not a lot of places where fixEncoding runs under python3 right now, pretty much just import-archive --type maven. So I wonder if import-archive works on py3 as-is, or if it would work better if fixEncoding returned str.
Tested import-archive --type maven with current master and https://pagure.io/fork/julian8628/koji/c/3d55e9d
They both work fine.
ping @mikem @tkopecek for reviewing this candidate
When rebased, this breaks one of the newer unit tests. I've made an adjustment to that test here: https://github.com/mikem23/koji-playground/commits/pagure/pr/578
This function works well enough, but I'm cautious about how we will want to approach this long term.
On my branch, I've renamed the function to _fix_print and added a docstring. I want to discourage clients from using this function for now. I think we'll end up revisiting this as we port more of the code to python3.
Also I removed the if not value case. So far, we're only using this is a place where we expect an actual string, so let's not get ahead of ourselves. We don't know what we want to do with other values yet, if anything.
If that looks ok to you, I can merge it tomorrow.
That's good. I've rebased your commit into this PR
rebased onto 3464adae2e8a42f5f1a4249d7478cbb0ffb418a3
1 new commit added
test_formatChangelog
+1
Commit 62d4d1d1 fixes this pull-request
Pull-Request has been merged by mikem
fix issue#577