Patchwork extensions: don't fail to load on Windows if path ends with a '\' (issue4187)

login
register
mail settings
Submitter Ed Morley
Date March 5, 2014, 9:34 a.m.
Message ID <5316EF88.2030702@mozilla.com>
Download mbox | patch
Permalink /patch/3841/
State Accepted
Commit c04459f959ffa306e518cf41daf5872b78d2ae23
Headers show

Comments

Ed Morley - March 5, 2014, 9:34 a.m.
# HG changeset patch
# User Ed Morley <emorley@mozilla.com>
# Date 1394011865 0
#      Wed Mar 05 09:31:05 2014 +0000
# Node ID b11c37b00ccd3cf3f6ebcfc0ac3d019f1ad36d1d
# Parent  5beb49fd5958cd46cb933a8bccf482b56eb55b18
extensions: use normpath to allow trailing '\' on Windows (issue4187)

Fixes same issue as 5c794e7331e7 but now works on Windows too.

With this patch a trailing backward slash won't prevent the extension from
being found on Windows, and we continue to support any combination of 
forward
and back slashes within the path.
Augie Fackler - March 5, 2014, 3:11 p.m.
On Mar 5, 2014, at 4:34 AM, Ed Morley <emorley@mozilla.com> wrote:

> # HG changeset patch
> # User Ed Morley <emorley@mozilla.com>
> # Date 1394011865 0
> #      Wed Mar 05 09:31:05 2014 +0000
> # Node ID b11c37b00ccd3cf3f6ebcfc0ac3d019f1ad36d1d
> # Parent  5beb49fd5958cd46cb933a8bccf482b56eb55b18
> extensions: use normpath to allow trailing '\' on Windows (issue4187)

Looks better, but oddly patch doesn't apply, even over the declared parent. Can you try send it again?

> 
> Fixes same issue as 5c794e7331e7 but now works on Windows too.
> 
> With this patch a trailing backward slash won't prevent the extension from
> being found on Windows, and we continue to support any combination of forward
> and back slashes within the path.
> 
> diff -r 5beb49fd5958 -r b11c37b00ccd mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -43,10 +43,10 @@ def find(name):
> 
> def loadpath(path, module_name):
>     module_name = module_name.replace('.', '_')
> -    path = util.expandpath(path)
> +    path = util.normpath(util.expandpath(path))
>     if os.path.isdir(path):
>         # module/__init__.py style
> -        d, f = os.path.split(path.rstrip('/'))
> +        d, f = os.path.split(path)
>         fd, fpath, desc = imp.find_module(f, [d])
>         return imp.load_module(module_name, fd, fpath, desc)
>     else:
Augie Fackler - March 5, 2014, 3:27 p.m.
On Mar 5, 2014, at 10:11 AM, Augie Fackler <raf@durin42.com> wrote:

> 
> On Mar 5, 2014, at 4:34 AM, Ed Morley <emorley@mozilla.com> wrote:
> 
>> # HG changeset patch
>> # User Ed Morley <emorley@mozilla.com>
>> # Date 1394011865 0
>> #      Wed Mar 05 09:31:05 2014 +0000
>> # Node ID b11c37b00ccd3cf3f6ebcfc0ac3d019f1ad36d1d
>> # Parent  5beb49fd5958cd46cb933a8bccf482b56eb55b18
>> extensions: use normpath to allow trailing '\' on Windows (issue4187)
> 
> Looks better, but oddly patch doesn't apply, even over the declared parent. Can you try send it again?

Rebuilt the patch, pushing (since the old edition broke a buildbot)


>> 
>> Fixes same issue as 5c794e7331e7 but now works on Windows too.
>> 
>> With this patch a trailing backward slash won't prevent the extension from
>> being found on Windows, and we continue to support any combination of forward
>> and back slashes within the path.
>> 
>> diff -r 5beb49fd5958 -r b11c37b00ccd mercurial/extensions.py
>> --- a/mercurial/extensions.py
>> +++ b/mercurial/extensions.py
>> @@ -43,10 +43,10 @@ def find(name):
>> 
>> def loadpath(path, module_name):
>>    module_name = module_name.replace('.', '_')
>> -    path = util.expandpath(path)
>> +    path = util.normpath(util.expandpath(path))
>>    if os.path.isdir(path):
>>        # module/__init__.py style
>> -        d, f = os.path.split(path.rstrip('/'))
>> +        d, f = os.path.split(path)
>>        fd, fpath, desc = imp.find_module(f, [d])
>>        return imp.load_module(module_name, fd, fpath, desc)
>>    else:
>
Ed Morley - March 5, 2014, 3:45 p.m.
On 05 March 2014 15:27:20, Augie Fackler wrote:
>> Looks better, but oddly patch doesn't apply, even over the declared parent. Can you try send it again?
>
> Rebuilt the patch, pushing (since the old edition broke a buildbot)

Thank you - sorry hadn't used patchbomb the second time around since I 
didn't want to break the email threading (not sure how the project 
keeps track of review comments against commits/Bugzilla, since the 
patches aren't in Bugzilla; presuming multiple threads makes it 
harder?) and Thunderbird must have mangled it.

Cheers,

Ed
Augie Fackler - March 5, 2014, 3:47 p.m.
On Mar 5, 2014, at 10:45 AM, Ed Morley <emorley@mozilla.com> wrote:

> On 05 March 2014 15:27:20, Augie Fackler wrote:
>>> Looks better, but oddly patch doesn't apply, even over the declared parent. Can you try send it again?
>> 
>> Rebuilt the patch, pushing (since the old edition broke a buildbot)
> 
> Thank you - sorry hadn't used patchbomb the second time around since I didn't want to break the email threading (not sure how the project keeps track of review comments against commits/Bugzilla, since the patches aren't in Bugzilla; presuming multiple threads makes it harder?) and Thunderbird must have mangled it.

patchbomb has an --in-reply-to flag where you can provide a message-id to preserve threading.

Multiple threads are also generally okay, though this time it was helpful.

Thanks!

> 
> Cheers,
> 
> Ed
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - March 5, 2014, 3:55 p.m.
On Mar 5, 2014, at 10:47 AM, Augie Fackler <raf@durin42.com> wrote:

> 
> On Mar 5, 2014, at 10:45 AM, Ed Morley <emorley@mozilla.com> wrote:
> 
>> On 05 March 2014 15:27:20, Augie Fackler wrote:
>>>> Looks better, but oddly patch doesn't apply, even over the declared parent. Can you try send it again?
>>> 
>>> Rebuilt the patch, pushing (since the old edition broke a buildbot)
>> 
>> Thank you - sorry hadn't used patchbomb the second time around since I didn't want to break the email threading (not sure how the project keeps track of review comments against commits/Bugzilla, since the patches aren't in Bugzilla; presuming multiple threads makes it harder?) and Thunderbird must have mangled it.
> 
> patchbomb has an --in-reply-to flag where you can provide a message-id to preserve threading.
> 
> Multiple threads are also generally okay, though this time it was helpful.
> 
> Thanks!

Buildbot still sad. The patch I pushed was:

http://hg.intevation.org/mercurial/crew/rev/dfd12cd88725

failure is:

http://hgbuildbot.kublai.com/builders/vfat%20hg%20tests/builds/156/steps/vfat/logs/stdio

halp?

(I can also just kill the change, if that's the right decision.)

> 
>> 
>> Cheers,
>> 
>> Ed
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>
Ed Morley - March 5, 2014, 4:10 p.m.
On 05 March 2014 15:55:48, Augie Fackler wrote:
> Buildbot still sad. The patch I pushed was:
>
> http://hg.intevation.org/mercurial/crew/rev/dfd12cd88725

Seems like a rebase typo? The patch in my email had:
> d, f = os.path.split(path)
rather than:
> d, f = os.path.split('/')

Thanks again for unmangling thunderbird's mess for me :-)

And cheers for the tip about |--in-reply-to|, I've added it to the wiki 
page for new contributors:
http://mercurial.selenic.com/wiki/ContributingChanges#Mailer_issues_and_patchbomb

Cheers,

Ed

Patch

diff -r 5beb49fd5958 -r b11c37b00ccd mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -43,10 +43,10 @@  def find(name):

  def loadpath(path, module_name):
      module_name = module_name.replace('.', '_')
-    path = util.expandpath(path)
+    path = util.normpath(util.expandpath(path))
      if os.path.isdir(path):
          # module/__init__.py style
-        d, f = os.path.split(path.rstrip('/'))
+        d, f = os.path.split(path)
          fd, fpath, desc = imp.find_module(f, [d])
          return imp.load_module(module_name, fd, fpath, desc)
      else: