[Aries][Boot] Fail to boot if built with Mac OS

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
3 years ago
8 months ago

People

(Reporter: wcpan, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

There are some error message ((BSD-style)gnutar does not support --owner=0, stat -c options ... etc.), but the building process still success.
After flash.sh, the phone is bricked.
Thomas, do you have patch for this? Thanks.
Flags: needinfo?(tnguyen)
Spent a little before to make it works. What I changed:
- Copy external/elfutils/libelf/elf.h to /usr/local/include/
- Apply attached patch.
It works for my Aries KK, I did not try Aries L
Thanks
Flags: needinfo?(tnguyen)
Created attachment 8701695 [details] [diff] [review]
0001-Fix-stat-size-in-darwin-system.patch

stat -c does not work for darwin system. We should use stat -f instead.
Thanks
So which coreutils should we use on darwin systems?
According to the document[1], it suggests us to use GNU coreutils instead of BSD.
But in this case (as well as flame-kk), I have to add a shim to force it use BSD stat instead of gstat.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Firefox_OS_build_prerequisites#Homebrew
Flags: needinfo?(lissyx+mozillians)
Blocks: 1194655
Flags: needinfo?(lissyx+mozillians)
I would prefer some solution similar to bug 1194677
Tested this patch with aries, this works for me.
(In reply to Wei-Cheng Pan [:wcp] [:wcpan] [:legnaleurc] from comment #6)
> Tested this patch with aries, this works for me.

As far as I can tell, it is not really expected to be able to build a linux kernel with BSD tools. Please avoid hacking within the kernel to change calls and fix the environment as I exposed in bug 1194677. We have tooling to prepare environment on OSX as far as I can tell (I'm not a Mac user), so we could be generating a .userconfig that includes PATH overriding for including GNU coreutils and others.
Created attachment 8702221 [details] [review]
Fix stat command

Looks like we should use GNU commands instead of BSD on Darwin systems.
Attachment #8702221 - Flags: review?(mwu)

Comment 9

3 years ago
Hmm, but on my (old 10.7) OSX system, stat -f seems to be correct... I wonder which OSX version this changed in.
Based on comment 7 and comment 8, the following steps work for my Mac:
- install gstat : brew install coreutils
- Create symlink: ln -s /usr/local/bin/gstat /usr/local/bin/stat
- export PATH="/usr/local/bin:$PATH" (we have to put /usr/local/bin in the head of PATH so the above symlink will be used)
- Apply Wei-Cheng's patch

Thanks
(In reply to Michael Wu [:mwu] from comment #9)
> Hmm, but on my (old 10.7) OSX system, stat -f seems to be correct... I
> wonder which OSX version this changed in.

stat -f %z is for Mac OS and BSD, but GNU coreutils can not use stat -f %z, and unfortunately some build scripts assume we are using GNU coreutils, this's why I made this patch.

Or maybe we can do some feature testing to decide the actual command, like:

if stat -c %s . 1>/dev/null 2>/dev/null; then
    size_cmd="stat -c %s"
elif stat -f %z . 1>/dev/null 2>/dev/null; then
    size_cmd="stat -f %z"
fi
I'm really not confident we should hack stat calls within the kernel itself ... I prefer something like what Thomas suggests in comment 10, even though I would prefer we avoid messing within /usr/local/bin and do that in a specific directory, like in bug 1194677 comment 1: we get GNU feature without making the local system default behavior changed.
Can we find a solution better, like Thomas suggested in comment 10 ?

If we can limit ourselves to: mac bootstrap script installing GNU stat, forcing that in the PATH during kernel build, then we should be able to do something in conjunction of bug 1234774
Flags: needinfo?(wpan)
Flags: needinfo?(tnguyen)
IMHO, installing something during a building process is slightly weird.
 
Maybe we can:
1. bundle a shell script which implements GNU stat, that just delegate to Mac's stat
2. (right before building kernel) backup $PATH, export PATH="/path/to/fake_stat:$PATH"
3. build kernel
4. restore $PATH
Flags: needinfo?(wpan)
(In reply to Wei-Cheng Pan [:wcp] [:wcpan] [:legnaleurc] from comment #14)
> IMHO, installing something during a building process is slightly weird.

Why? There is already config.sh step and Mac bootstrap script. I'm just saying we should handle this at this level.

>  
> Maybe we can:
> 1. bundle a shell script which implements GNU stat, that just delegate to
> Mac's stat
> 2. (right before building kernel) backup $PATH, export
> PATH="/path/to/fake_stat:$PATH"
> 3. build kernel
> 4. restore $PATH

You are describing what I said in comment 13 ...
(In reply to Alexandre LISSY :gerard-majax from comment #15)
> Why? There is already config.sh step and Mac bootstrap script. I'm just
> saying we should handle this at this level.

Sorry I missed that part, I thought we are tending to do that in the build stage.

> You are describing what I said in comment 13 ...

OK, I just want to make sure I'm not misunderstanding what you are going to do.

It looks fine to me.
Sorry for the late. Agree with solution. 
We can add gstat installation to https://github.com/mozilla-b2g/B2G/blob/master/scripts/bootstrap-mac.sh
But I am not sure where should be good to add stat symlink to the PATH in kernel (probably Makefile, IMHO).
Will take a look later on this if I have time. If anyone is interested in this bug, please feel free to take it
Flags: needinfo?(tnguyen)
(In reply to Thomas Nguyen[:tnguyen][:thomas][:nguyen] from comment #10)
> Based on comment 7 and comment 8, the following steps work for my Mac:
> - install gstat : brew install coreutils
> - Create symlink: ln -s /usr/local/bin/gstat /usr/local/bin/stat
> - export PATH="/usr/local/bin:$PATH" (we have to put /usr/local/bin in the
> head of PATH so the above symlink will be used)
> - Apply Wei-Cheng's patch
> 
> Thanks

These steps worked for me as well
Can you do a PR for the missing steps ? Thanks !
Flags: needinfo?(apastor)
Created attachment 8742893 [details] [review]
https://github.com/albertopq/B2G/pull/1

Don't know a lot about this script, but is this what you meant?
Flags: needinfo?(apastor)
Attachment #8742893 - Flags: review?(tnguyen)
(In reply to Alberto Pastor [:albertopq] from comment #20)
> Created attachment 8742893 [details] [review]
> https://github.com/albertopq/B2G/pull/1
> 
> Don't know a lot about this script, but is this what you meant?

Yep, this is it :)
Attachment #8742893 - Flags: review?(tnguyen) → review?(lissyx+mozillians)
Comment on attachment 8742893 [details] [review]
https://github.com/albertopq/B2G/pull/1

I am sorry, but I don't have a mac, I don't know the bootstrap script, and I am not knowledgeable about the whole homebrew stuff. I don't see how I can seriously review this.
Attachment #8742893 - Flags: review?(lissyx+mozillians) → review?(tnguyen)
Comment on attachment 8742893 [details] [review]
https://github.com/albertopq/B2G/pull/1

I am sorry :(, I think I'm not the right person to say yea or no on this :(. I am not currently working on b2g.
Maybe you can try to ping another person, probably :mwu
Thanks
Attachment #8742893 - Flags: review?(tnguyen)
(In reply to Thomas Nguyen[:tnguyen][:thomas][:nguyen] from comment #23)
> Comment on attachment 8742893 [details] [review]
> https://github.com/albertopq/B2G/pull/1
> 
> I am sorry :(, I think I'm not the right person to say yea or no on this :(.
> I am not currently working on b2g.
> Maybe you can try to ping another person, probably :mwu
> Thanks

But you have a mac and you hacked on this. So you can have a much better opinion.
Flags: needinfo?(tnguyen)
It makes sense to me to add missing gstat package. However you probably need to link "stat" to "gstat" to use gstat. Otherwise system will continue use wrong stat with wrong argument.
Thanks
Flags: needinfo?(tnguyen)

Comment 26

8 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.