Re: [PATCH] Fix Cygwin path handling

[Vladimir 'φ-coder/phcoder' Serbinenko]

Christian Franke wrote:
> Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> Christian Franke wrote:
>>   
>>> The Cywin path handling is broken since
>>> make_system_path_relative_to_its_root() functionality was moved from
>>> the lib script to misc.c.
>>>
>>> This patch should fix this. It reuses the Cygwin specific code from
>>> getroot.c:grub_get_prefix() which apparently is a different
>>> implementation of the same function.
>>>
>>> I would suggest to remove grub_get_prefix(), it is now only used in
>>> grub-emu.c and sparc64/ieee1275/grub-setup.c. Not included in the
>>> patch, should be done in a separate commit.
>>>
>>>
>>> 2010-04-14  Christian Franke<address@hidden>
>>>
>>>      * util/grub-mkconfig_lib.in
>>> (make_system_path_relative_to_its_root):
>>>      Remove broken Cygwin path conversion.
>>>      * util/misc.c: [__CYGWIN__] Add include and define.
>>>      [__CYGWIN__] (get_win32_path): Copy function from getroot.c,
>>> modify
>>>      for Cygwin 1.7.
>>>      
>> Please avoid duplicating code. Rather than that rename get_win32_path to
>> grub_get_win32_path and remove static attribute
>>    
>
> Normally I would have done that but duplication was intentional in
> this case:
> The getroot.c:get_win32_path() can later be removed together with
> grub_get_prefix(), see my suggestion above. The patch takes this into
> account and adds new private misc.c:get_win32_path() and so avoids
> unnecessary temporary changes to misc.h and getroot.c.
>
> The actual code duplication happened when
> misc.c:make_system_path_relative_to_its_root() was added instead of
> moving and reusing getroot.c:grub_get_prefix() :-)
>
Ok. Can you supply the dedup patch? (perhaps it should come before the fix).
>
> BTW: My last commits to grub codebase were before the move to bzr.
>
> As far as I understand "Bazaar workflow for GRUB"
> (http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00175.html)
> such changes should be 'bzr push'ed to e.g. '.../branches/feature-foo'
> (e.g. '.../branches/cygwin-path' in this case) after review has finished.
>
Creating new branches doesn't need any approval at all. If the changes
are approved for trunk they are applied or merged into trunk.
experimental branch is a merge of sufficiently functional branches but
which need more testing for testers convenience. Merging into it follows
similar rules as comitting to trunk.
> Is this workflow still valid or is there a more current document?
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature