Closed Bug 1426244 Opened 2 years ago Closed 2 years ago

Automatically generate and upload javadoc for GeckoView

Categories

(GeckoView :: General, enhancement)

All
Android
enhancement
Not set

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(5 files, 1 obsolete file)

No description provided.
Thanks! I'm working on some patches to add Github loading.
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 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.
Attachment #8939579 - Flags: review?(nalexander) → review+
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.
Attachment #8939581 - Flags: review?(nalexander) → review+
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 :)
Attachment #8939583 - Flags: review?(nalexander) → review-
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?
Attachment #8939582 - Flags: review?(nalexander) → review-
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 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 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.
Attachment #8939582 - Flags: review?(nalexander) → review+
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.
Attachment #8939583 - Flags: review?(nalexander) → review+
Comment on attachment 8939580 [details]
Bug 1426244 - 2. Fix javadoc comments;

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

Looks good to me.
Attachment #8939580 - Flags: review?(droeh) → review+
Depends on: 1429222
Attachment #8939581 - Attachment is obsolete: true
Pushed by nchen@mozilla.com:
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
Use the new variant 'officialWithGeckoBinariesNoMinApiRelease', which is
used by other m-c build jobs, for building javadoc.
Attachment #8942974 - Flags: review+
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 59 → mozilla59
See Also: → 1539250
You need to log in before you can comment on or make changes to this bug.