Closed Bug 897727 Opened 11 years ago Closed 6 years ago

B2G emulator build requires both 32-bit and 64-bit libgl, but Debian's libgl1-mesa-dev package is not multilib compatible

Categories

(Firefox OS Graveyard :: GonkIntegration, defect)

x86_64
Linux
defect
Not set
blocker

Tracking

(firefox31-)

RESOLVED WONTFIX
Tracking Status
firefox31 - ---

People

(Reporter: justin.lebar+bug, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

Debian's libgl1-mesa-dev package is not multilib compatible.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689088

This means that you can't have the 32-bit and 64-bit versions of libgl1-mesa-dev installed on your machine at the same time.

Unfortunately this is what our emulator build wants.

In order to build the emulator I had to switch back and forth between the 32-bit and 64-bit packages.  Who knows if the binary will even work once it finishes building.

If we support the emulator as a build target, we need to fix this.
Prioritization datapoint: this is a HUGE productivity problem for me as it means that I can't run reftests on my regular development machine (running reftest on device fails for me, too, so I really need the emulator).

Blocks my work on bug 758845.

Anyone knows a linux distro with multilib GL and X libraries?
Blocks: 758845
It seems that the reason why both 32bit and 64bit versions of the GL and X libraries are required, is that the emulator build-system wants to build both 32bit and 64bit versions of everything.

I manually removed the 64bit sections of the Android.mk files in all the subdirs of the emulator tree; that seems to allow my build to continue for now.

Questions:
 - why would the emulator's build system want to build both 32bit and 64bit executables?
 - any specific reason why the ./run-emulator.sh script is running the 32bit version ('emulator') as opposed to the 64bit one ('emulator64') ?
Flags: needinfo?(mwu)
Oh, got something actually running. With my above setup building only 32bit binaries, I still had the problem that, contrary to what I claimed in comment 3 (from a misreading of the source), ./run-emulator.sh tried running the 64bit emulator. Making symlinks so that it'd actually run the 32bit one, I got it to start (also with some LD_LIBRARY_PATH to make it find my 32bit libGL.so.1).

Next up, let me try to see if I can get the _64bit_ emulator to work...
Flags: needinfo?(mwu)
(In reply to Benoit Jacob [:bjacob] from comment #3)
>  - why would the emulator's build system want to build both 32bit and 64bit
> executables?

My guess is that's because it's building for redistribution, not for local use.

Changing the rules to build only the native version (32-bits on 32-bits systems, 64-bits on 64-bits systems) would be good.
This works here (ubuntu 12.04 64bit) and ./run-emulator.sh just works. (Thanks to Gabriele Svelto for help)
Attachment #786280 - Flags: review?(vyang)
I have no such problem ever.  Here are mesa* packages installed in my chroot:

  ii  libgl1-mesa-dev                 9.1.1-0ubuntu3                   amd64
  ii  libgl1-mesa-glx:amd64           9.1.1-0ubuntu3                   amd64
  ii  libgl1-mesa-glx:i386            9.1.1-0ubuntu3                   i386
  ii  libglapi-mesa:amd64             9.1.1-0ubuntu3                   amd64
  ii  libglapi-mesa:i386              9.1.1-0ubuntu3                   i386
  ii  mesa-common-dev                 9.1.1-0ubuntu3                   amd6

Besides, `/usr/lib/x86_64-linux-gnu/{,mesa/}libGL.so` were installed by libgl1-mesa-dev, and `/usr/lib/i386-linux-gnu/{,mesa/}libGL.so` are manually linked to `mesa/libGL.so.1`.
Comment on attachment 786280 [details] [diff] [review]
Change the emulator's build system to build only 32bit or only 64bit according to the host architecture

Just manually create symbolic links in /usr/lib/i386-linux-gnu.
Attachment #786280 - Flags: review?(vyang)
Hi, Vicamo.

bjacob and I have both wasted a lot of time trying to build the emulator because of this issue.

If the solution you want as owner (?) of this code is to require us to manually create symlinks, then I'd ask that you please write a patch to change the build failure so that it prints a helpful error message.

Our first job as maintainers of code is to avoid wasting our coworkers' time.  Not making this issue easy to work around -- either with an actual fix or with a helpful error message -- will just continue to waste peoples' time, and that's not OK.

I also don't understand what's wrong with this patch.  It seems to obviate the need for a non-obvious hack, and I doubt that printing a helpful error message will be much easier than the approach here.  Could you please elaborate on what's wrong with bjacob's approach?
Flags: needinfo?(vyang)
(In reply to Justin Lebar [:jlebar] (limited availability 8/9 – 8/12) from comment #9)
> Hi, Vicamo.
> 
> bjacob and I have both wasted a lot of time trying to build the emulator
> because of this issue.

Hmmm, we don't have a Emulator component, or I would certainly help on this issue at the time I see this bug.

> If the solution you want as owner (?) of this code is to require us to
> manually create symlinks, then I'd ask that you please write a patch to
> change the build failure so that it prints a helpful error message.

Yes, just add a few more lines in |B2G/README.md| will do.

> Our first job as maintainers of code is to avoid wasting our coworkers'
> time.  Not making this issue easy to work around -- either with an actual
> fix or with a helpful error message -- will just continue to waste peoples'
> time, and that's not OK.
> 
> I also don't understand what's wrong with this patch.  It seems to obviate
> the need for a non-obvious hack, and I doubt that printing a helpful error
> message will be much easier than the approach here.  Could you please
> elaborate on what's wrong with bjacob's approach?

Because:
1) That touches too much Android.mk code.  During ICS -> JB transition, we just have to create a new branch for external/qemu because it has many external dependencies and just doesn't go well with ICS Gonk.  Since we have emulator-jb now, the work here has to be completely re-evaluated/re-do again before you can build emulator-jb.  In contrast, creating symbolic links works for both ICS/JB.

2) As I have said, we have already a really simple fix for this.  Like all other steps listed in B2G/README.md, the thing we need is to add a few lines in that file, telling people that they'll have to manually create some symbolic links just like they'll have to manually install some packages before being able to compile B2G.
Flags: needinfo?(vyang)
> Yes, just add a few more lines in |B2G/README.md| will do.

No, that's not the same as a helpful error message when the build fails (or before it fails).

* The readme is already long; it's unreasonable to expect everyone to read it.
* Even if I did read the readme once, I'm not going to go looking through it when my build fails, in the hopes that someone put a hint in it for me.

There is no greater responsibility of a module owner than to ensure that people don't waste their time fighting with breakage caused by the component you own.  Putting something in the readme and calling it a day simply isn't good enough.  The time bjacob, I, and others have wasted and will continue to waste here until we fix this properly is too precious.

Can you write a script which runs before the build and checks that the symlinks are there?
Assignee: nobody → vyang
Severity: normal → blocker
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7)
> I have no such problem ever.  Here are mesa* packages installed in my chroot:
> 
>   ii  libgl1-mesa-dev                 9.1.1-0ubuntu3                   amd64
>   ii  libgl1-mesa-glx:amd64           9.1.1-0ubuntu3                   amd64
>   ii  libgl1-mesa-glx:i386            9.1.1-0ubuntu3                   i386
>   ii  libglapi-mesa:amd64             9.1.1-0ubuntu3                   amd64
>   ii  libglapi-mesa:i386              9.1.1-0ubuntu3                   i386
>   ii  mesa-common-dev                 9.1.1-0ubuntu3                   amd6

Hah, I see. I tried to install both amd64 and i386 versions of libgl1-mesa-dev, but that is the package that is not multilib compatible. I see that you only installed both versions of libgl1-mesa-glx and libglapi-mesa, which is indeed enough, as the only thing that we need is the shared library, we don't need the headers.

I also understand that having custom patches against the Android build system is not great.

But if we take the route for addressing this only at the documentation level, there are a few things to worry about:

 1. There is no single one documentation resource for building B2G: The README.md is what some people go to, while other people (like me) used to go to the MDN wiki for documentation:
 https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Firefox_OS_build_prerequisites
So maybe unifying the documentation around building B2G is a prerequisite before we can content ourselves with resolving this issue at the documentation level.

 2. This is quite difficult to explain: the explanation depends on the linux distribution, and in the case of debian/ubuntu 64bit, one has to explain that one must install libgl1-mesa-dev, libglapi-mesa:i386 and libgl1-mesa-glx:i386 but *NOT* libgl1-mesa-dev:i386 (which would uninstall libgl1-mesa-dev, causing unnecessary trouble elsewhere). Then one still has to tell people to create symlinks. If other distros also have multilib limitations around GL, they will require explanations too...
I updated this wiki page:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Firefox_OS_build_prerequisites

I looked at the README.md in the B2G repo and only found very minimalistic explanations. I think that it makes more sense to stick to keep all the nontrivial build docs on the MDN wiki, and only have the README.md refer to it.
Sorry, Justin, I still can't feel the same with you.  Gonk source build depends on many native libraries, and libGL is just one of them.  I feel no reason that we should treat it a special case and write some checker for it.  People want to build Gonk from source have to prepare that build environment, install packages, etc.  For those don't even want to read the manuals or do all necessary steps before executing |./build.sh|, I feel no responsibility to them.
Assignee: vyang → nobody
If you own this, it's your call.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
I and Andrea individually wasted some time fighting the build issues about this bug recently.

Right now, emulator-jb builds fine out of the box.  I don't think that requiring people to symlink is acceptable here, when we can have an in-tree fix which just works for everybody here.

Vicamo, does the ICS->JB transition issue still exist?  If not, can we just take this patch please?

Thanks!
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
All the host emulator binaries are required in PRODUCT_PACKAGES[1] list of all emulator variants, so that patch alone still doesn't work for emulator-jb.

[1]: https://github.com/mozilla-b2g/platform_build/blob/b2g-4.3_r2.1/target/product/emulator.mk#L21
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #17)
> Vicamo, does the ICS->JB transition issue still exist?  If not, can we just
> take this patch please?

Besides, I think there is no "ICS->JB transition" actually.  We're going to have both of them for a while(?).  So that patch, or patches, must be applied to both ICS and JB qemu.
(In reply to comment #19)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #17)
> > Vicamo, does the ICS->JB transition issue still exist?  If not, can we just
> > take this patch please?
> 
> Besides, I think there is no "ICS->JB transition" actually.  We're going to
> have both of them for a while(?).  So that patch, or patches, must be applied
> to both ICS and JB qemu.

But the JB emulator is *not* affected by this bug.  Why would we want to patch that?
Also, note that Boris had filed this 7 months ago as well (bug 847553).  We *really* need to fix this.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #20)
> But the JB emulator is *not* affected by this bug.  Why would we want to
> patch that?

Ok, I had another chroot built from scratch and found even we do have i386 emulator binaries built, we don't need i386 libgl to run JB emulator.  I'll try to merge that patch into ICS emulator.
Comment on attachment 786280 [details] [diff] [review]
Change the emulator's build system to build only 32bit or only 64bit according to the host architecture

Review of attachment 786280 [details] [diff] [review]:
-----------------------------------------------------------------

::: emulator/opengl/host/libs/GLESv1_dec/Android.mk
@@ +6,4 @@
>  #host_common_debug_CFLAGS += -DCHECK_GL_ERROR
>  #host_common_debug_CFLAGS += -DDEBUG_PRINTOUT
>  
> +ifeq ($(shell uname -m), x86_64)

Sorry, don't know if you're still willing to maintain this patch.  But, could you use `ifneq` instead?  Just want to have minimum changeset here.  Don't move code blocks around.
Have a GitHub PR in https://github.com/mozilla-b2g/android-sdk/pull/6 .  It's for branch 'emu-fix', which is our current tracking branch of B2G trunk.

For B2G v1.2, we use branch 'v1.2' and it happens to be on the same commit with 'emu-fix'.  We may uplift this to v1.2 with a simple forwarding merge.  I've verified the patch here for both B2G trunk and B2G v1.2 on a Debian Sid chroot.

For B2G v1-train and older, we're tracking AOSP tags and have a pinned revision instead.  Not tested.
Thanks a lot, Vicamo!  :-)
Any updates here?
Flags: needinfo?(vyang)
Flags: needinfo?(vyang) → needinfo?(bjacob)
Sorry, I've completely lost track of this bug, and I don't have a B2G-emulator checkout at the moment... what should I answer?
Flags: needinfo?(bjacob)
Looks like we have (or had) a patch that was close to fixing this but it never got landed due to confusion of ownership. Can we sort out who needs to do what? Vicamo, are you waiting for Benoit to update his patch? Benoit, are you washing your hands of this bug? If so who can drive it?
Flags: needinfo?(vyang)
Flags: needinfo?(bjacob)
if you don't mind, I can help have a revision (and merge that? depends whether do I have the privilege to that repository)
Flags: needinfo?(vyang)
Too many other things came up, and knowing that this build-system issue was already fixed upstream in JellyBean, so that the issue is already fixed is you configure for emulator-jb and only happens if you configure for (ICS) emulator, and applying review comments and verifying the modified patch is a little bit tedious to do, this never was a priority.
Flags: needinfo?(bjacob)
Finally, I needed to be able to build the emulator again, so here is a new version of the patch addressing your above review comments to minimize the diff size.
Attachment #8383025 - Flags: review?(vyang)
Attachment #786280 - Attachment is obsolete: true
(I was hoping that I would only ever need to build emulator-jb, but it seems that ICS emulator still works better, and is still what we do regression testing against).
I'm a little bit curious why qemu is not involved here and is doing some experiments to remove the depending modules from the build list instead.  Will give an answer here in one or two days.  Thank you!
(In reply to Benoit Jacob [:bjacob] from comment #32)
> (I was hoping that I would only ever need to build emulator-jb, but it seems
> that ICS emulator still works better, and is still what we do regression
> testing against).

It's likely we'll only get a small subset of tests running against emulator-jb. We'll be targeting the most of the rest for emulator-kk instead. Unfortunately this issue isn't going to "just go away" anytime soon, so thanks for updating the patch!
Somehow I find Kitkat & JellyBean still build libEGL_translator.so and other 32-bit libraries, but they leave "libGL.so.1" slot not found instead of exiting with errors.

ICS/JB/KK invoke ld with completely identical parameters, but that "ld" itself varies.  JB/KK are using "prebuilts/tools/gcc-sdk/gcc", while ICS uses system CC/CXX.  If we just cherry-pick an upstream change [1][2] to build/, then everything works.  In this way, we can also drop build dependency to g++, gcc, and gcc-multilib.  The thing we only need is libstdc++6:i386.

Except Gecko doesn't compile.  Can't find x86_64-linux-gnu-gcc.  Must be some $PATH problem.  Should already have a solution in GECKO/configure.in, I bet.

Besides, although prebuilts/tools/ is available in both emulator and emulator-x86, but the target folder build/ itself is shared by all ICS devices, inclusive of those don't have "prebuilts/tools".  [1] must come with prebuilts/tools/, and polluting all device manifests doesn't really sound a good idea.

So, in comparison to all above problems, the patch here looks much more simple and elegant than a couple hours ago.  Thank you!

[1]: https://android.googlesource.com/platform/build.git/+/4e82d1fa7f3fb1ecfa6cbd8b8ddcb6c0c0e17d1d
[2]: in order to allow overriding host toolchain, also need: https://android.googlesource.com/platform/build.git/+/79e3f7799648f6de27087f17b2b50cc79870805e
Attachment #8383025 - Flags: review?(vyang) → review+
https://github.com/mozilla-b2g/android-sdk/commit/4f97016504df559e0ed25f5f867e820e882cdbf8
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
Glad to be of help and thanks for investigating this and integrating this patch!
seems this caused https://tbpl.mozilla.org/php/getParsedLog.php?id=35658278&tree=B2g-Inbound which is currently closing the tree. Trying to find someone to back this out since my try was not successful via git
I am going to block the 31 release for this bug. It has been existing for a while and does not impact directly firefox.
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 10 years ago6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: