1428989 - AndroidRect.{cpp,h} are not deterministically generated

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

There are multiple methods with the same name and that differ in their arguments. They end up being ordered in the source file randomly, despite there being some sorting done, because the sorting is only done on the method name.

This yields differences in the generated libxul.so, such as those that can be seen in https://public-artifacts.taskcluster.net/JTNlfog1TRqo6Ky_2R6lXQ/0/public/diff.html

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1428989 - Generate Android bindings more deterministically.

There are multiple methods with the same name and that differ in their
arguments. They end up being ordered in the source file randomly,
despite there being some sorting done, because the sorting was only done
on the method name.

Now, when the method name matches, also compare the arguments.

Review commit: https://reviewboard.mozilla.org/r/211252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211252/

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8940974 [details]
Bug 1428989 - Generate Android bindings more deterministically.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211252/diff/1-2/

Comment 3

[Mike Hommey [:glandium]]

Diff after this patch: https://queue.taskcluster.net/v1/task/FLAhdaVPTQWzC_gF7EH3SA/runs/0/artifacts/public/diff.html

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8940974 [details]
Bug 1428989 - Generate Android bindings more deterministically.

This appears to not be enough.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8940974 [details]
Bug 1428989 - Generate Android bindings more deterministically.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211252/diff/2-3/

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8940974 [details]
Bug 1428989 - Generate Android bindings more deterministically.

https://reviewboard.mozilla.org/r/211252/#review217178

Thanks, glandium.

::: build/annotationProcessors/SDKProcessor.java:339
(Diff revision 3)
>              @Override
>              public int compare(Member a, Member b) {
> -                return a.getName().compareTo(b.getName());
> +                int result = a.getName().compareTo(b.getName());
> +                if (result == 0) {
> +                    if (a instanceof Constructor && b instanceof Constructor) {
> +                        String sa = Arrays.toString(((Constructor)a).getParameterTypes());

nit: space after the cast, like `(Constructor) a` (throughout).

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8940974 [details]
Bug 1428989 - Generate Android bindings more deterministically.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211252/diff/3-4/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5faa09169991
Generate Android bindings more deterministically. r=nalexander

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/5faa09169991