All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Franke <Christian.Franke@t-online.de>
To: The development of GNU GRUB <grub-devel@gnu.org>
Subject: Re: [PATCH] Fix Cygwin path handling
Date: Sat, 17 Apr 2010 18:10:39 +0200	[thread overview]
Message-ID: <4BC9DD7F.3030701@t-online.de> (raw)
In-Reply-To: <4BC9D233.9090307@gmail.com>

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<franke@computer.org>
>>
>>      * 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() :-)


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.

Is this workflow still valid or is there a more current document?

-- 
Regards,
Christian Franke




  reply	other threads:[~2010-04-17 16:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-14 18:36 [PATCH] Fix Cygwin path handling Christian Franke
2010-04-17 15:22 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-04-17 16:10   ` Christian Franke [this message]
2010-04-17 20:58     ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-04-19 14:49     ` BVK Chaitanya
2010-04-26  1:45 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-04-26  6:12   ` Christian Franke
2010-04-26  6:56     ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-04-26 10:18       ` Christian Franke
2010-05-01 19:40         ` Vladimir 'φ-coder/phcoder' Serbinenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BC9DD7F.3030701@t-online.de \
    --to=christian.franke@t-online.de \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.