736463 - javac argument list too long when compiling third-party Java libraries

Status: RESOLVED INVALID

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Nathan Toone [:toonetown]]

Attachment: log of make which caused failure.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_3) AppleWebKit/535.18.5 (KHTML, like Gecko) Version/5.2 Safari/535.18.5

Steps to reproduce:

Tried compiling Fennec 11 on Ubuntu 64-bit


Actual results:

Compilation failed when compiling third-party java files in http client.  Failure was "Argument List too long".  Attaching the make log file.


Expected results:

Compilation should have succeeded.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Heh. Most people building this will probably not be able to reproduce it unless their build directory's absolute path length is >= strlen("/home/ntoone/Documents/k9/android/browser-lib/target/")

(I've run into this sort of a problem at my previous company, where builds from release branches would fail because the path would have a few extra characters and push it over the limit).

In the meantime a workaround might be to hard-link your mozilla checkout to somewhere in $HOME so you have a shorter path and build from there.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

We've seen this before, in other modules. Its unfortunate, but the work-around Kats suggested works fine.

Comment 3

[Richard Newman [:rnewman]]

To be fair, we are putting 930 Java classes in a single line, producing a 131,756-character string.

The answer here is probably to adjust SYNC_THIRDPARTY_JAVA_FILES:

	$(JAVAC) $(JAVAC_FLAGS) -d classes $(addprefix $(srcdir)/,$(SYNC_THIRDPARTY_JAVA_FILES))

and do something like this instead:

	$(JAVAC) $(JAVAC_FLAGS) -d classes "@../path/to/sync_thirdparty_java_source.mn"

which will instruct javac to read the filenames from our manifest, rather than messing around with Makefile variables and a single command string.

I think that makes sense, and I'm happy to put together a patch. blassey?

Comment 4

[Nathan Toone [:toonetown]]

I thought of doing that by wasn't quite sure it would work since the files references in the mn file are relative, and the build directory is different. As I have a reproducible case though, I'm willing to try something out too.

Comment 5

[Richard Newman [:rnewman]]

(In reply to Nathan Toone from comment #4)
> I thought of doing that by wasn't quite sure it would work since the files
> references in the mn file are relative, and the build directory is
> different. As I have a reproducible case though, I'm willing to try
> something out too.

If you mean "willing to do some Makefile hackery", please feel free! Any non-machine-specific transform to the manifest contents (e.g., making it multi-line, adjusting the relative paths therein) is acceptable.

If you mean "willing to try the result", then I'll keep you posted.

Comment 6

[Nathan Toone [:toonetown]]

I will play around with the makefile a bit - however, I'm a make n00b, so I don't know how much progress I'll make.  I'll keep you posted.

Comment 7

[Richard Newman [:rnewman]]

Going to reopen this, because it is an issue that ought to be -- and in this case can be! -- fixed, albeit at a low priority.

If Nathan doesn't get to it, I will.

Comment 8

[Nathan Toone [:toonetown]]

It looks like the quick fix is beyond my abilities...I think that the .mn file will need to be preprocessed to include the full path...because when loading java source files from a file, they need to be relative to the working directory - which it isn't in this case.

Comment 9

[Nathan Toone [:toonetown]]

Don't know how (or if) this has been fixed, but using the latest code from mozilla-central, I do not see this anymore.

Comment 10

[Richard Newman [:rnewman]]

(In reply to Nathan Toone from comment #9)
> Don't know how (or if) this has been fixed, but using the latest code from
> mozilla-central, I do not see this anymore.

Not fixed; most likely we've just had a net reduction in number of files in the past week, and so you've dipped back under the limit.

Either that or you upgraded your kernel or shell or some other component that was limiting your argument list length.

Still in issue in general.

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: generate manifesets to pass to javac

This patch generates manifests (only when the data sources change) that have full paths to the java source files based on information from the makefile java file lists and the current working directory.  There is a slight overhead for generating the manifests, but that only needs to be done when the makefiles change.  The manifests need to be generated at build time because the srcdir and topsrcdir are only known at that point and they need to be in the manifests.

I have a try job pending, but I have tested this with dep builds locally a lot.  This approach compiles the same number of files during a dep build.

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

https://tbpl.mozilla.org/?tree=Try&rev=5dd9fb8dc782

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

it's worth mentioning that the echo command is equally likely to fail, since it's only trivially shorter an argv than the one to javac.  Breaking up the var list into smaller argv lists to build the manifest is probably the easiest way to fix this, assuming that all java files need to be compiled together.

Comment 14

[Richard Newman [:rnewman]]

Is there a reason why you aren't using the manifests we generate during import? That seems like the most obvious solution here, even if you'd need to push them through sed.

Perhaps so you can track the modifications for make's dependencies?

And what's wrong with using -sourcepath rather than needing to include the source directories in the paths directly?

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 623207 [details] [diff] [review]
generate manifesets to pass to javac

r- until response to Comment 14. Keepin' dem review queues quiet!

Comment 16

[Nick Alexander :nalexander [he/him]]

Let me add this myself

Comment 17

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #16)
> Let me add this myself

Sorry, let me add myself to this.  I missed a ping from IRC user `adh` about this; my apologies!

Comment 18

[Nick Alexander :nalexander [he/him]]

Technical details:

mobile/android/base/Makefile.in includes mobile/android/base/android-sync-files.mk.  That file is generated by https://github.com/mozilla-services/android-sync/blob/develop/fennec-copy-code.sh.

There are two parts to this ticket.

1.  Determine how to pass a file list (which I'll call a manifest) to javac, and slice the SYNC_*_FILES variables out of android-sync-files.mk and into said manifest files.  This step can be done without any reference to the android-sync code; the patch should just be updating Makefile.in and rewriting stuff.

2.  Update fennec-copy-code.sh to write the updated android-sync-files.mk and new manifest files.  You should be able to git clone android-sync, update this script, and test:

$ cd android-sync
$ ./fennec-copy-code.sh ../mozilla-inbound

without setting up a full android-sync build environment.

A patch for at least part 1 would be great.

Comment 19

[Nick Alexander :nalexander [he/him]]

A word to the wise: you should be able to test this stuff with just

make -C $OBJDIR/mobile/android/base && make -C $OBJDIR package

That will regenerate Makefile from Makefile.in and re-package everything.

Comment 20

[Richard Newman [:rnewman]]

Note that we already have source manifests, produced by our code drop script, and have done since January:

  mobile/android/sync/java-*sources.mn

These should be suitable for passing to javac.

See Comment 14.

Comment 21

[Nick Alexander :nalexander [he/him]]

Hi Alex, didn't want to let this slip through the cracks.  Any progress?  Any questions?

Comment 22

[Alexander Hagerman]

Nick,

Sorry I'm getting this issue and was trying to work around it.

remoteuser@alexhagerman-optiplex-760:/home/remoteuser/Downloads/src/mobile/android/base$ make -C /home/remoteuser/Downloads/src/mobile/android/base && make -C /home/remoteuser/Downloads/src package
make: Entering directory `/home/remoteuser/Downloads/src/mobile/android/base'
make: *** No targets specified and no makefile found.  Stop.
make: Leaving directory `/home/remoteuser/Downloads/src/mobile/android/base'

Comment 23

[Nick Alexander :nalexander [he/him]]

(In reply to alex.hagerman from comment #22)
> Nick,
> 
> Sorry I'm getting this issue and was trying to work around it.
> 
> remoteuser@alexhagerman-optiplex-760:/home/remoteuser/Downloads/src/mobile/
> android/base$ make -C /home/remoteuser/Downloads/src/mobile/android/base &&
> make -C /home/remoteuser/Downloads/src package
> make: Entering directory `/home/remoteuser/Downloads/src/mobile/android/base'
> make: *** No targets specified and no makefile found.  Stop.
> make: Leaving directory `/home/remoteuser/Downloads/src/mobile/android/base'

I think I see the problem.  The way this works is that the Makefile.ins live in src/foo/bar and get turned into Makefiles in objdir/foo/bar as part of building.

You'll need to first do:

src $ make -f client.mk

that will build all the platform stuff (and Fennec).  You'll have the Makefile's in objdir after this, and I think they're smart enough to recognize when the corresponding Makefile.in changes.  So you'll be able to edit

src/mobile/android/base/Makefile.in

and rebuild with:

src $ make -C objdir/mobile/android/base

(Observe the objdir there.)  This is confusing, but see where you get to.

Comment 24

[Alexander Hagerman]

That initial make took a while (and on a side note I had to run it as sudo?)

However I'm still receiving this 

error:remoteuser@alexhagerman-optiplex-760:~/Downloads/src$ make -C /home/remoteuser/Downloads/src/mobile/android/base
make: Entering directory `/home/remoteuser/Downloads/src/mobile/android/base'
make: *** No targets specified and no makefile found.  Stop.
make: Leaving directory `/home/remoteuser/Downloads/src/mobile/android/base'

Comment 25

[Nick Alexander :nalexander [he/him]]

(In reply to alex.hagerman from comment #24)
> That initial make took a while (and on a side note I had to run it as sudo?)

Hmm, this is odd.  Why?  You should not need root (after all prerequisites are installed).  You could look at https://developer.mozilla.org/en-US/docs/Simple_Firefox_build for a better starting experience.

> However I'm still receiving this 
> 
> error:remoteuser@alexhagerman-optiplex-760:~/Downloads/src$ make -C
> /home/remoteuser/Downloads/src/mobile/android/base
> make: Entering directory `/home/remoteuser/Downloads/src/mobile/android/base'
> make: *** No targets specified and no makefile found.  Stop.
> make: Leaving directory `/home/remoteuser/Downloads/src/mobile/android/base'

I wonder if you actually built Fennec.  Can you verify that your .mozconfig contains

ac_add_options --enable-application=mobile/android

Did you follow the instructions at https://wiki.mozilla.org/Mobile/Fennec/Android?

Comment 26

[Richard Newman [:rnewman]]

(In reply to alex.hagerman from comment #24)

> error:remoteuser@alexhagerman-optiplex-760:~/Downloads/src$ make -C
> /home/remoteuser/Downloads/src/mobile/android/base

The make -C needs to change into your object directory, not the source directory. E.g.,

  make -C objdir-droid/mobile/android/base

Comment 27

[Alexander Hagerman]

My mozconfig is below:

# Add the correct paths here:
ac_add_options --with-android-ndk="$HOME/Downloads/android-ndk-r5c"
ac_add_options --with-android-sdk="$HOME/Downloads/android-sdk-linux/platforms/android-16"
ac_add_options --with-android-toolchain="$HOME/Downloads/android-ndk-r5c/toolchains/arm-linux-androideabi-4.4.3/prebuilt/linux-x86"
ac_add_options --with-android-version=5


# android options
ac_add_options --enable-application=mobile/android
ac_add_options --target=arm-linux-androideabi
ac_add_options --with-ccache
ac_add_options --enable-tests

mk_add_options MOZ_OBJDIR=./objdir-droid
mk_add_options MOZ_MAKE_FLAGS="-j9 -s"

rnewman - thanks so much that fixed the build now I can test some changes I've been making. Thank you.

Comment 28

[Nick Alexander :nalexander [he/him]]

Further to IRC conversation with adh:

13:22 nalexander: adh: victory!
13:22 nalexander: http://docs.oracle.com/javase/1.4.2/docs/tooldocs/windows/javac.html
13:22 nalexander: @argfiles

So we can include the manifests from the command line.

Comment 29

[Richard Newman [:rnewman]]

(In reply to Nick Alexander :nalexander from comment #28)

> 13:22 nalexander: @argfiles
> 
> So we can include the manifests from the command line.

You mean like I said in Comment 3?

Comment 30

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #29)
> (In reply to Nick Alexander :nalexander from comment #28)
> 
> > 13:22 nalexander: @argfiles
> > 
> > So we can include the manifests from the command line.
> 
> You mean like I said in Comment 3?

You win a small prize.

Comment 31

[Nick Alexander :nalexander [he/him]]

adh: no need to loop in with me, post a patch and ask for feedback (f?) from me or rnewman.

Comment 32

[Nick Alexander :nalexander [he/him]]

Status update: Java is both maximally helpful -- it supports @argfiles -- and maximally unhelpful -- each file in said @argfile needs to either have an absolute path or be relative to the current directory.  In the context of Fennec's build process, PWD=objdir-droid/mobile/android/base, which means that @argfile entries would need to look like

../../../../sync/Foo.java

There is no guarantee that $OBJDIR is anywhere close to $(topsrcdir), so statically set relative @argfile entries are not possible.  Absolute @argfile entries conflict with every revision control system.  Dynamically generated @argfiles are possible.

Comment 33

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Closing this – we intend to move to gradle so I don't think this is relevant anymore.