1426244 - Automatically generate and upload javadoc for GeckoView

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Comment 1

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&selectedJob=151873226 is my WIP on this.  It sets up an android-docs job but doesn't push to Github pages.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Thanks! I'm working on some patches to add Github loading.

Comment 3

[Nick Alexander :nalexander [he/him]]

Just FYI, the Gecko whitelist thing moved -- put it in https://searchfox.org/mozilla-central/source/taskcluster/ci/config.yml#83 now.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1426244 - 1. Fix some errors from javadoc generation;

Tweak the javadoc task arguments to not generate errors for a standard
GeckoView javadoc run.

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

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1426244 - 2. Fix javadoc comments;

Add some missing lines and fix some mistakes in our javadoc comments.

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

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1426244 - 3. Fix CustomTabsSecurityPopup package;

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

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1426244 - 4. Add mach android geckoview-docs command;

Add a './mach android geckoview-docs' command that generates javadoc for
GeckoView using Gradle. Specifying the '--archive' option will generate
a geckoview-javadoc.jar archive. Specifying the '--upload' option,
intended primarily for Taskcluster, will upload the javadoc to Github
pages under the given user/repository.

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

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1426244 - 5. Add geckoview-docs job;

Add a gv-docs job that executes "./mach android geckoview-docs", which
takes care of calling gradle to generate the javadoc archive, and
uploading it to Github using given parameters.

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

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

https://reviewboard.mozilla.org/r/209900/#review215470

::: testing/mozharness/configs/builds/releng_sub_android_configs/64_gv_docs.py:13
(Diff revision 1)
> +    'disable_package_metrics': True,
> +    'postflight_build_mach_commands': [
> +        ['android',
> +         'geckoview-docs',
> +         '--archive',
> +         '--upload', 'darchons/geckoview', # FIXME

This will be changed to a Mozilla repo

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939579 [details]
Bug 1426244 - 1. Fix some errors from javadoc generation;

https://reviewboard.mozilla.org/r/209892/#review215570

This is a bit of a stamp, but I'm happy for you to own this configuration entirely.  Roll on!

::: mobile/android/geckoview/build.gradle:192
(Diff revision 1)
> -        classpath = files(variant.javaCompile.classpath.files) + files(android.bootClasspath)
> -        options.windowTitle("Mozilla GeckoView ${mozconfig.substs.MOZ_APP_VERSION} API Reference")
> -        options.docTitle("Mozilla GeckoView ${mozconfig.substs.MOZ_APP_VERSION}")
> -        options.header("Mozilla GeckoView ${mozconfig.substs.MOZ_APP_VERSION} API Reference")
> -        options.bottom("© 2016 Mozilla. All rights reserved.")
> -        options.links("http://docs.oracle.com/javase/7/docs/api/")
> +        exclude '**/R.java', '**/BuildConfig.java', 'com/google/**', 'org/webrtc/**'
> +        options.addPathOption('sourcepath', ':').setValue(
> +            variant.sourceSets.collect({ it.javaDirectories }).flatten() +
> +            variant.generateBuildConfig.sourceOutputDir)
> +
> +        // javadoc 8 has a bug that requires the rt.jar

Link to reference for this, please.

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939581 [details]
Bug 1426244 - 3. Fix CustomTabsSecurityPopup package;

https://reviewboard.mozilla.org/r/209896/#review215572

Thanks for this, I saw it and was confused a while back.

Comment 12

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

https://reviewboard.mozilla.org/r/209900/#review215576

r- just for me to take a second look after mass renaming.

Overall, I think this is looking really good.  Thanks, Jim!

::: commit-message-3d337:3
(Diff revision 1)
> +Bug 1426244 - 5. Add gv-docs job; r?nalexander
> +
> +Add a gv-docs job that executes "./mach android geckoview-docs", which

This really a nit, but let's make our lives easier and use geckoview-docs everywhere except the TH symbol.  That is, I want grepping for geckoview-docs to hit everything, and not just half the things.  So make the job names, etc, geckoview-docs as well.

::: taskcluster/docker/android-build/Dockerfile
(Diff revision 1)
>  CMD ["/bin/bash", "--login"]
>  
>  # Set apt sources list to a snapshot.
>  COPY sources.list /etc/apt/
>  
> -# We need i386 packages for the Android SDK.

This is really a Pre:, but whatever.

::: testing/mozharness/mozharness/mozilla/building/buildbase.py:448
(Diff revision 1)
>          'aarch64': 'builds/releng_sub_%s_configs/%s_aarch64.py',
>          'android-test': 'builds/releng_sub_%s_configs/%s_test.py',
>          'android-checkstyle': 'builds/releng_sub_%s_configs/%s_checkstyle.py',
>          'android-lint': 'builds/releng_sub_%s_configs/%s_lint.py',
>          'android-findbugs': 'builds/releng_sub_%s_configs/%s_findbugs.py',
> +        'android-gv-docs': 'builds/releng_sub_%s_configs/%s_gv_docs.py',

Observe that `android-geckoview-docs` will correspond to |mach android geckoview-docs|... that's not an accident for the other ones :)

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

https://reviewboard.mozilla.org/r/209898/#review215574

r- to give me a chance to think about the security and certificate stuff more after you respond, and for you to explain the division of environment and command line parameters.

::: mobile/android/mach_commands.py:319
(Diff revision 1)
> +        help='Upload generated javadoc to Github, using '
> +             'the gh-pages branch of the specified USER/REPO.')
> +    @CommandArgument('--variant', default='debug',
> +        help='Gradle variant used to generate javadoc.')
> +    def android_geckoview_docs(self, archive, upload, variant):
> +        task = 'geckoview:javadoc' + ('Jar' if archive or upload else '') + (

nit: extract a `capitalize` helper please, following https://searchfox.org/mozilla-central/source/mobile/android/gradle.configure#42

::: mobile/android/mach_commands.py:326
(Diff revision 1)
> +        ret = self.gradle([task], verbose=True)
> +        if ret or not upload:
> +            return ret
> +
> +        # Upload to Github.
> +        fmt = {

We're taking a lot from the environment.  Is it possible to take these things from the command line and push the coupling between command and automation closer to the Task Cluster definitions?

::: mobile/android/mach_commands.py:344
(Diff revision 1)
> +            keyfile = mozpath.abspath('gv-docs-upload-key')
> +            with open(keyfile, 'w') as f:
> +                os.chmod(keyfile, 0o600)
> +                f.write(req.json()['secret']['content'])
> +
> +            env['GIT_SSH_COMMAND'] = 'ssh -i "%s" -o StrictHostKeyChecking=no' % keyfile

Explain to me what's going on here.  Is this a thing with github pages using a * certifacte or something?

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939579 [details]
Bug 1426244 - 1. Fix some errors from javadoc generation;

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

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

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

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939581 [details]
Bug 1426244 - 3. Fix CustomTabsSecurityPopup package;

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

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

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

Comment 18

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

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

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

https://reviewboard.mozilla.org/r/209898/#review215574

> We're taking a lot from the environment.  Is it possible to take these things from the command line and push the coupling between command and automation closer to the Task Cluster definitions?

I think it would make sense to make `GECKOVIEW_DOCS_UPLOAD_PREFIX` and `GECKOVIEW_DOCS_UPLOAD_MESSAGE` into command line arguments. For the other env vars, they really only make sense for a taskcluster environment, so I think they would make less sense as general-purpose command line arguments.

> Explain to me what's going on here.  Is this a thing with github pages using a * certifacte or something?

`StrictHostKeyChecking=no` is used to turn off the "Authenticity of the host can't be established" prompt from ssh, because ssh inside the container doesn't know about the github.com host [1]. We can keep a copy of gihub's host key somewhere to workaround this, but I don't think we need that because we are not pushing anything sensitive here.

[1] http://bencane.com/2013/07/22/ssh-disable-host-checking-for-scripts-automation/

Comment 20

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

https://reviewboard.mozilla.org/r/209898/#review215574

> I think it would make sense to make `GECKOVIEW_DOCS_UPLOAD_PREFIX` and `GECKOVIEW_DOCS_UPLOAD_MESSAGE` into command line arguments. For the other env vars, they really only make sense for a taskcluster environment, so I think they would make less sense as general-purpose command line arguments.

OK, that's fine with me.

> `StrictHostKeyChecking=no` is used to turn off the "Authenticity of the host can't be established" prompt from ssh, because ssh inside the container doesn't know about the github.com host [1]. We can keep a copy of gihub's host key somewhere to workaround this, but I don't think we need that because we are not pushing anything sensitive here.
> 
> [1] http://bencane.com/2013/07/22/ssh-disable-host-checking-for-scripts-automation/

I concur, there's nothing sensitive here.  Include a note to explain why you're doing this, please -- somebody else will cult this and we might as well educate :)

Comment 21

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

https://reviewboard.mozilla.org/r/209894/#review216334

Stamp!

Comment 22

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

https://reviewboard.mozilla.org/r/209898/#review216336

r+ so you don't have to do another review loop, but I do expect you to address the helper and the comment that I asked for.

Comment 23

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

https://reviewboard.mozilla.org/r/209900/#review216338

r+ since this doesn't need another review cycle, but it does need some details about what the secret is and how to produce a replacement secret when GH requires all tokens to be updated.

::: mobile/android/mach_commands.py:320
(Diff revision 2)
>               'the gh-pages branch of the specified USER/REPO.')
>      @CommandArgument('--variant', default='debug',
>          help='Gradle variant used to generate javadoc.')
>      def android_geckoview_docs(self, archive, upload, variant):
> -        task = 'geckoview:javadoc' + ('Jar' if archive or upload else '') + (
> -                                      variant[0].upper() + variant[1:] if variant else '')
> +
> +        def capitalize(s):

Ah -- it got to here.  Better to fold it into the first patch.

::: testing/mozharness/configs/builds/releng_sub_android_configs/64_geckoview_docs.py:13
(Diff revision 2)
> +    'disable_package_metrics': True,
> +    'postflight_build_mach_commands': [
> +        ['android',
> +         'geckoview-docs',
> +         '--archive',
> +         '--upload', 'darchons/geckoview', # FIXME

I don't think we should land with this.

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939579 [details]
Bug 1426244 - 1. Fix some errors from javadoc generation;

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

Comment 25

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

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

Comment 26

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939581 [details]
Bug 1426244 - 3. Fix CustomTabsSecurityPopup package;

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

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

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

Comment 28

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

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

Comment 29

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

https://reviewboard.mozilla.org/r/209894/#review217266

Looks good to me.

Comment 31

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939579 [details]
Bug 1426244 - 1. Fix some errors from javadoc generation;

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

Comment 32

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

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

Comment 33

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939581 [details]
Bug 1426244 - 3. Fix CustomTabsSecurityPopup package;

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

Comment 34

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

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

Comment 35

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

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

Comment 36

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939579 [details]
Bug 1426244 - 1. Fix some errors from javadoc generation;

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

Comment 37

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

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

Comment 38

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939582 [details]
Bug 1426244 - 4. Add mach android geckoview-docs command;

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

Comment 39

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8939583 [details]
Bug 1426244 - 5. Add geckoview-docs job;

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

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/516d427dcc01
1. Fix some errors from javadoc generation; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/14ba5b4ea2ab
2. Fix javadoc comments; r=droeh,nalexander
https://hg.mozilla.org/integration/autoland/rev/70e6e0af0064
4. Add mach android geckoview-docs command; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/ca369a31f46c
5. Add geckoview-docs job; r=nalexander

Comment 41

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/516d427dcc01
https://hg.mozilla.org/mozilla-central/rev/14ba5b4ea2ab
https://hg.mozilla.org/mozilla-central/rev/70e6e0af0064
https://hg.mozilla.org/mozilla-central/rev/ca369a31f46c

Comment 42

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 6. Follow-up to use new variant (v1)

Use the new variant 'officialWithGeckoBinariesNoMinApiRelease', which is
used by other m-c build jobs, for building javadoc.

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0137e00cf221
Follow-up to use new variant; r=me

Comment 44

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0137e00cf221

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a98033301da
Follow-up to use notimestamp; r=me

Comment 46

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/3a98033301da