Re: Split patch for Mac OS/ was Re: Help with Building toolchain with crosstool-ng on Mac OS X 10.6.2 (Snow Leopard)

["Yann E. MORIN" <yann.morin.1998@anciens.enib.fr>]

Hello Titus, All!

On Friday 19 February 2010 07:15:25 Titus von Boxberg wrote:
> >> -has_or_abort prog="awk gawk" ver='^GNU Awk' err="GNU 'awk' was not found"
> >> +has_or_abort prog="gawk awk" ver='^GNU Awk' err="GNU 'awk' was not found"
> > Huh? Why do you need to switch the check ?
> As far as I remember has_or_abort did not check for the 2nd prog name, and
> the linux boxes in my company have gawk as well, so: pure lazyness ;-)
> awk is not called by ctng (at least not in incompatible
> ways) and many if not all tool configures look for gawk, anyway.
> Can insert that into the GNU wrapper --with-xx-list as well.

Well, if awk is indeed GNU awk, there's no need to test the second arg, of
course! So, this is a purely gratuituous change. Dropped.

> > [--SNIP--]
> >> -has_or_abort prog=stat ver='GNU coreutils'
> >> +has_or_abort prog=stat
> > 
> > One separate patch for that, please. I want to be able to revert just this
> > in case it breaks later, whithout reverting the whole patch.
> I kindly but strongly object.
> This patch by itself does not and can not break anything for
> GNU systems because stat really is a *core*util. I cannot think of a
> nonbroken GNU/Linux system where stat is not from GNU.

busybox' stat... Although that one supports the same formats as coreutils'.

And a system running the Linux kernel and a busybox-based userland is not
a GNU/Linux system, and I would not call that broken.

> I would even recommend to delete the line to make it consistent
> with readlink not being checked for as well, and because I
> cannot think of any nonbroken system without stat.
> What do you think of this?

I'd say that we should check for readlink also. This is an omission.

> > --> sed-portable:
> > Note however that there are many other places where sed is called with -r.
> > Did you make sure those where not impacted also?
> I'm pretty sure that this is the only location where the links to GNU
> tools have not yet been set up.

Yes. You're right. The other places use the wrapper. Thanks for pointing
this out!

> > --> stat-portable:
> > 
> >> diff --git a/scripts/build/internals.sh b/scripts/build/internals.sh
[--SNIP--]
> > And separate patch for that, please, as it's not related to the rest
> > of the patch.
> It is related because it replaces a non portable call to stat
> which is what the rest of this patch does, too.

Yes, but it fixes two different issues with stat. At least 'internals.sh'
and 'functions' can be in the same patch, but the populate hunks (below)
should be a separate patch.

> >> +            echo "OS unknown";
> > Redirect to stderr!
> CT_DoLog maybe?

Not available in populate...

> >> -src_inode=$(stat -c '%i' "${CT_ROOT_SRC_DIR}/.")
> >> -dst_inode=$(stat -c '%i' "${CT_ROOT_DST_DIR}/." 2>/dev/null || true)
> >> +src_inode=$(CT_getinode "${CT_ROOT_SRC_DIR}/.")
> >> +dst_inode=$(CT_getinode "${CT_ROOT_DST_DIR}/." 2>/dev/null || true)
> > 
> > Oh well, stderr is consigned to oblivion here, which means you wont see
> > the "OS unknown" error message anyway...
> > 
> >> if [ "${src_inode}" -eq "$((dst_inode+0))" ]; then
> >>     echo "$myname: source and destination are the same!"
> >>     exit 1
> > Separate patch, please.
> > Seems we can do way better here: move the '2>/dev/null || true' up into
> > the function.
> Imo such a function with a name suggesting general usability
> should normally complain visibly when the file was not found.
> So I would opt to leave the /dev/null where it is.
> Or didn't I get the message right?

Sorry, I was not clear enough. I meant smthg like:

CT_getinode() {
    case "$(uname -s)" in
        *BSD|Darwin) stat -f '%i' $1 2>/dev/null || true;;
        Linux)       stat -c '%i' $1 2>/dev/null || true;;
        *)           echo "OS unknown" >&2; exit 1;;
    esac
}

src_inode=$(CT_getinode "${CT_ROOT_SRC_DIR}/.")
dst_inode=$(CT_getinode "${CT_ROOT_DST_DIR}/.")

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
`------------------------------^-------^------------------^--------------------'



--
For unsubscribe information see http://sourceware.org/lists.html#faq