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
next prev parent 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.