[1/3] python extended prompt: Use os.getcwd() instead of os.getcwdu()

Message ID 1416976561-1927-1-git-send-email-simon.marchi@ericsson.com
State Committed
Headers

Commit Message

Simon Marchi Nov. 26, 2014, 4:35 a.m. UTC
  It seems like using os.getcwdu() here is wrong both for Python 2 and Python 3.

For Python 2, this returns a 'unicode' object, which tries to get concatenated
to a 'str' object in substitute_prompt. The implicit conversion works when the
unicode string contains no accent. When it does contain an accent though,
displaying the prompt results in the following error:

(gdb) set extended-prompt \w
...
  File "/home/simark/build/binutils-gdb-python2/gdb/data-directory/python/gdb/prompt.py", line 138, in substitute_prompt
    result += str(cmd(arg))
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 49: ordinal not in range(128)

When using os.getcwd() instead, it works correctly. I suppose that Python does
the necessary decoding internally.

For Python 3, this method simply does not exist. It works fine with os.getcwd().

gdb/ChangeLog:

	* python/lib/gdb/prompt.py (_prompt_pwd): Use os.getcwd() instead of
	os.getcwdu().
---
 gdb/python/lib/gdb/prompt.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Joel Brobecker Nov. 27, 2014, 9 a.m. UTC | #1
> It seems like using os.getcwdu() here is wrong both for Python 2 and Python 3.
> 
> For Python 2, this returns a 'unicode' object, which tries to get concatenated
> to a 'str' object in substitute_prompt. The implicit conversion works when the
> unicode string contains no accent. When it does contain an accent though,
> displaying the prompt results in the following error:
> 
> (gdb) set extended-prompt \w
> ...
>   File "/home/simark/build/binutils-gdb-python2/gdb/data-directory/python/gdb/prompt.py", line 138, in substitute_prompt
>     result += str(cmd(arg))
> UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 49: ordinal not in range(128)
> 
> When using os.getcwd() instead, it works correctly. I suppose that Python does
> the necessary decoding internally.
> 
> For Python 3, this method simply does not exist. It works fine with os.getcwd().
> 
> gdb/ChangeLog:
> 
> 	* python/lib/gdb/prompt.py (_prompt_pwd): Use os.getcwd() instead of
> 	os.getcwdu().

I'd like to have other people's opinion on this, as I am not sure.

I _think_ that the patch is not making things worse for us,
while making things a little better in situations as the above.
So, based on that, I'd be inclined to apply it.

However, I think the long term fix would be, I believe, to switch
the entire thing to unicode. With Python3, it's automatic, but
with Python2, we might have to add 'u'-s on every piece of string
in the module, and also add some conversions here and there.
That's why I am thinking that the long term fix should be a blocker
for this patch.

Thoughts?

> ---
>  gdb/python/lib/gdb/prompt.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/python/lib/gdb/prompt.py b/gdb/python/lib/gdb/prompt.py
> index d99f2ea..04adbfb 100644
> --- a/gdb/python/lib/gdb/prompt.py
> +++ b/gdb/python/lib/gdb/prompt.py
> @@ -21,7 +21,7 @@ import os
>  
>  def _prompt_pwd(ignore):
>      "The current working directory."
> -    return os.getcwdu()
> +    return os.getcwd()
>  
>  def _prompt_object_attr(func, what, attr, nattr):
>      """Internal worker for fetching GDB attributes."""
> -- 
> 2.1.3
  
Simon Marchi Nov. 28, 2014, 3:49 p.m. UTC | #2
> I'd like to have other people's opinion on this, as I am not sure.
> 
> I _think_ that the patch is not making things worse for us,
> while making things a little better in situations as the above.
> So, based on that, I'd be inclined to apply it.
> 
> However, I think the long term fix would be, I believe, to switch
> the entire thing to unicode. With Python3, it's automatic, but
> with Python2, we might have to add 'u'-s on every piece of string
> in the module, and also add some conversions here and there.
> That's why I am thinking that the long term fix should be a blocker
> for this patch.
>
> Thoughts?

An eventual switch to use unicode everywhere would certainly undo this
patch. However, I don't see the point in leaving the broken code as-is,
unless there are imminent plans to make that switch happen.

Simon
  
Joel Brobecker Nov. 29, 2014, 11:42 a.m. UTC | #3
> > I'd like to have other people's opinion on this, as I am not sure.
> > 
> > I _think_ that the patch is not making things worse for us,
> > while making things a little better in situations as the above.
> > So, based on that, I'd be inclined to apply it.
> > 
> > However, I think the long term fix would be, I believe, to switch
> > the entire thing to unicode. With Python3, it's automatic, but
> > with Python2, we might have to add 'u'-s on every piece of string
> > in the module, and also add some conversions here and there.
> > That's why I am thinking that the long term fix should be a blocker
> > for this patch.
> >
> > Thoughts?
> 
> An eventual switch to use unicode everywhere would certainly undo this
> patch. However, I don't see the point in leaving the broken code as-is,
> unless there are imminent plans to make that switch happen.

That's exactly my thinking. But not being very familiar with this
area, and the handling of unicode, I would like to give others
an opportunity to jump in. Let's wait another week, and see if
we get additional feedback. If not, we'll push the patch.
  
Simon Marchi Dec. 5, 2014, 8:36 p.m. UTC | #4
On 2014-11-29 06:42 AM, Joel Brobecker wrote:
>>> I'd like to have other people's opinion on this, as I am not sure.
>>>
>>> I _think_ that the patch is not making things worse for us,
>>> while making things a little better in situations as the above.
>>> So, based on that, I'd be inclined to apply it.
>>>
>>> However, I think the long term fix would be, I believe, to switch
>>> the entire thing to unicode. With Python3, it's automatic, but
>>> with Python2, we might have to add 'u'-s on every piece of string
>>> in the module, and also add some conversions here and there.
>>> That's why I am thinking that the long term fix should be a blocker
>>> for this patch.
>>>
>>> Thoughts?
>>
>> An eventual switch to use unicode everywhere would certainly undo this
>> patch. However, I don't see the point in leaving the broken code as-is,
>> unless there are imminent plans to make that switch happen.
> 
> That's exactly my thinking. But not being very familiar with this
> area, and the handling of unicode, I would like to give others
> an opportunity to jump in. Let's wait another week, and see if
> we get additional feedback. If not, we'll push the patch.

All right, if nothing comes up about this, I'll push it Monday, first thing
in the morning, while eating my cereals.
  
Simon Marchi Dec. 15, 2014, 4:41 p.m. UTC | #5
On 2014-12-05 03:36 PM, Simon Marchi wrote:
> On 2014-11-29 06:42 AM, Joel Brobecker wrote:
>>>> I'd like to have other people's opinion on this, as I am not sure.
>>>>
>>>> I _think_ that the patch is not making things worse for us,
>>>> while making things a little better in situations as the above.
>>>> So, based on that, I'd be inclined to apply it.
>>>>
>>>> However, I think the long term fix would be, I believe, to switch
>>>> the entire thing to unicode. With Python3, it's automatic, but
>>>> with Python2, we might have to add 'u'-s on every piece of string
>>>> in the module, and also add some conversions here and there.
>>>> That's why I am thinking that the long term fix should be a blocker
>>>> for this patch.
>>>>
>>>> Thoughts?
>>>
>>> An eventual switch to use unicode everywhere would certainly undo this
>>> patch. However, I don't see the point in leaving the broken code as-is,
>>> unless there are imminent plans to make that switch happen.
>>
>> That's exactly my thinking. But not being very familiar with this
>> area, and the handling of unicode, I would like to give others
>> an opportunity to jump in. Let's wait another week, and see if
>> we get additional feedback. If not, we'll push the patch.
> 
> All right, if nothing comes up about this, I'll push it Monday, first thing
> in the morning, while eating my cereals.

I just pushed this.

Thanks.
  

Patch

diff --git a/gdb/python/lib/gdb/prompt.py b/gdb/python/lib/gdb/prompt.py
index d99f2ea..04adbfb 100644
--- a/gdb/python/lib/gdb/prompt.py
+++ b/gdb/python/lib/gdb/prompt.py
@@ -21,7 +21,7 @@  import os
 
 def _prompt_pwd(ignore):
     "The current working directory."
-    return os.getcwdu()
+    return os.getcwd()
 
 def _prompt_object_attr(func, what, attr, nattr):
     """Internal worker for fetching GDB attributes."""