1032615 - Build fails using zsh to build geckoview_resources.zip.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chris Kitching [:ckitching]]

When building using zsh, the following error is encountered after Proguard completes:

../../../dist/bin/nsinstall: cannot access geckoview_resources.zip: No such file or directory

The file does not exist when the libs target runs, despite being marked as a dependency.

The rule for building geckoview_resources.zip is:

# We delete the archive before updating so that resources removed from
# the filesystem are removed from the archive.
geckoview_resources.zip: $(all_resources) $(GLOBAL_DEPS)
	$(REPORT_BUILD)
	$(RM) -rf $@
	$(foreach dir,$(ANDROID_RES_DIRS),$(call zip_directory_with_relative_paths,$(CURDIR)/$@,$(dir)))


Under zsh, the removal works, but the command to construct it silently fails.
The zip command expands to something like:

zip -q /home/chris/Fennec/Fennec/objdir-droid/mobile/android/base/geckoview_resources.zip -r * -x *.mkdir.done* *.DS_Store* *#* *.rej *.orig

The asterisks in the argument list to -x should be escaped, else the shell may attempt to expand them.
This seems to be a recommended approach - from the `zip' manpage:

       -x files
       --exclude files
              Explicitly exclude the specified files, as in:

                     zip -r foo foo -x \*.o

              which  will  include  the  contents of foo in foo.zip while excluding all the files that end in .o.  The backslash avoids the shell filename substitution, so that the name matching is performed by zip at all directory levels.


I hadn't realised zsh was used by so few people for this to have not yet been noticed!

Comment 1

[Chris Kitching [:ckitching]]

Attachment: Prevent wildcard expansion from killing zsh.

A simple patch to fix the problem. You'll probably want a neater solution, though hopefully this concisely expresses precisely where the problem is.

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8448450 [details] [diff] [review]
Prevent wildcard expansion from killing zsh.

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

I'm a little bit surprised that we're hitting shell incompatibilities, but obviously, you are :)  I'd like to test against sh/bash just to be sure -- you could test the zip command manually -- and then bombs away.

::: mobile/android/base/Makefile.in
@@ +279,5 @@
>  
>  # $(1): zip file to add to (or create).
>  # $(2): directory to zip contents of.
>  define zip_directory_with_relative_paths
> +cd $(2) && zip -q $(1) -r * -x $(subst *, \\*, $(not_android_res_files))

Whitespace counts, so that should be

... (subst *,\\*,$(not_android_res_files))

(no extra spaces).  You could also add a new, hard-coded, not_android_res_files_patterns or similar.

Comment 3

[Chris Kitching [:ckitching]]

(In reply to Nick Alexander :nalexander from comment #2)
> I'm a little bit surprised that we're hitting shell incompatibilities, but
> obviously, you are :)  I'd like to test against sh/bash just to be sure --
> you could test the zip command manually -- and then bombs away.

Already did so for bash/zsh. Tried it also on dash and fish to make sure.

> Whitespace counts, so that should be...

Whoops.

Anyhoo, landed:
https://hg.mozilla.org/integration/fx-team/rev/767d3a8e6491


... Now I can actually build at last. :P

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/767d3a8e6491