Add hermetic macos support

[TendTo]

[ closes #4 ]

[TendTo]

Hey @kaycebasques, you motivated me into looking a bit more in the issue and, after a bit of refactor inspired by your fork, I produced a solution that satisfies me.
Let me know if this is what you needed or if there is any other changes you would like to implement before merging it in the main branch.

[kaycebasques]

Oh wow! I just submitted my non-working attempt haha #7

Let me try yours

[kaycebasques]

Almost working for us!!

need to specify different SHAs for different OSes

One thing not working is that doxygen_extension.version() seems to need to accept different SHAs for Mac, Windows, and Linux. Currently it's this:

doxygen_extension = use_extension("@rules_doxygen//:extensions.bzl", "doxygen_extension")
doxygen_extension.version(
    # linux_sha256 = "8354583f86416586d35397c8ee7e719f5aa5804140af83cf7ba39a8c5076bdb8",
    # mac_sha256 = "25ad21fa767bd71208947b848d482b46c6bc523a97ba1aacf803c71c4d043726",
    sha256 = "8354583f86416586d35397c8ee7e719f5aa5804140af83cf7ba39a8c5076bdb8",
    version = "1.9.6",
)

I set it to Linux SHA and it works on Linux. But when I run on Mac I get the "wanted SHA ... but actually got SHA ..." error

But when I set it to Mac SHA and run on Mac it works! So your change seems to work for both Mac and Linux

need to not clobber lock file

We use MODULE.bazel.lock files. In my implementation (#7) I was struggling with the lock file getting clobbered. E.g. I run on Mac and the lockfile gets updated for Mac SHAs but then our Linux CI/CQ bots fail. So it seems like the Doxygen repo rule needs to somehow save the Mac/Windows/Linux SHAs separately (or maybe separate repo rules for each OS?) I think this is where the lockfile is having problems:

    },
    "@@rules_doxygen+//:extensions.bzl%doxygen_extension": {
      "general": {
        "bzlTransitiveDigest": "qIud8MWwhexxfMnwEPGAYR3jtIq3IPxx0ZgLJfSAtUE=",
        "usagesDigest": "62PgImEiF1hn0K5idOh5gVcQU93o0HMr1kJzmQNY7Ro=",
        "recordedFileInputs": {},
        "recordedDirentsInputs": {},
        "envVariables": {},
        "generatedRepoSpecs": {
          "doxygen": {
            "bzlFile": "@@rules_doxygen+//:extensions.bzl",
            "ruleClassName": "doxygen_repository",
            "attributes": {
              "sha256": "8354583f86416586d35397c8ee7e719f5aa5804140af83cf7ba39a8c5076bdb8",
              "version": "1.9.6"
            }
          }
        },

Notice how there's only one sha256 saved for doxygen_repository

To repro in Pigweed:

1. git clone https://pigweed.googlesource.com/pigweed/pigweed
2. cd pigweed
3. git fetch https://pigweed.googlesource.com/pigweed/pigweed refs/changes/92/247192/49 && git checkout -b change-247192 FETCH_HEAD
4. bazelisk build //docs

(this one should work on linux but not on mac)

Doxygen-built DMG doesn't seem to support Apple ARM Silicon

(we can possibly address this one later, just noting now before I forget)

Our CI/CQ tryjobs (presubmits) seem to suggest that the Doxygen-provided DMG was built on an Intel Mac and doesn't actually support Arm-based Apple Silicon. I'm testing locally on a 2021 MacBook with M1 Pro so I don't know how my local builds are passing. Maybe Rosetta was silently enabled on my loaner laptop.

Note the Bad CPU type in executable in the logs

Analyzing: 2653 targets (515 packages loaded, 1164 targets configured)
[3 / 3] no actions running
ERROR: /Volumes/Work/s/w/ir/x/w/co/docs/BUILD.bazel:142:8: Running doxygen failed: (Exit 1): doxygen failed: error executing Action command (from target //docs:doxygen_build) 
  (cd /Volumes/Work/s/w/ir/cache/bazel/_bazel_chrome-bot/b8d148453ab029c90d2464c812e12339/sandbox/darwin-sandbox/1/execroot/_main && \
  exec env - \
  external/rules_doxygen++doxygen_extension+doxygen/doxygen bazel-out/darwin_arm64-fastbuild/bin/docs/Doxyfile)
# Configuration: ab0950038518105fef6a0763ce977a80d2bb684e7f31b73a1084d630f5213651
# Execution platform: @@platforms//host:host

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
src/main/tools/process-wrapper-legacy.cc:80: "execvp(external/rules_doxygen++doxygen_extension+doxygen/doxygen, ...)": Bad CPU type in executable
INFO: Elapsed time: 4.719s, Critical Path: 0.37s
INFO: 2 processes: 2 internal.
ERROR: Build did NOT complete successfully
FAILED: 

(feedback for later) remove non-hermetic options

I personally got bamboozled by the option to use a non-hermetic Doxygen binary. Pigweed has a workflow that puts Doxygen binary on my path that I forgot about. It would be better for us to just remove the non-hermetic binary option and always force hermetic build (in keeping with Bazel philosophy) to prevent accidents like what I went through

[TendTo]

Thank you for your insightful comments.

1. need to specify different SHAs for different OSes
   True, basically I did not account for running using the same MODULE.bazel on multiple platforms, except for the default settings.
   I would like to introduce the possibility of defining multiple extensions for different OSs, but I'm not sure this feature is supported. Otherwise, multiple sha256 values should solve the issue.

# My idea. I'm still not sure how it would work exactly

doxygen_extension = use_extension("@rules_doxygen//:extensions.bzl", "doxygen_extension")

doxygen_extension.version(
    sha256 = "25ad21fa767bd71208947b848d482b46c6bc523a97ba1aacf803c71c4d043726",
    version = "1.9.6",
   platform="macos"
)
doxygen_extension.version(
    sha256 = "8354583f86416586d35397c8ee7e719f5aa5804140af83cf7ba39a8c5076bdb8",
    version = "1.9.6",
   platform="linux"
)

use_repo(doxygen_extension, "doxygen")

2. Doxygen-built DMG doesn't seem to support Apple ARM Silicon
   The dmg is taken from doxygen's the official releases and I though that was the right executable. I have no experience with macos, so I'm not aware if there are alternative sources or I simply did not find the right binary in the folder.

3. remove non-hermetic options
   I think this option should still remain available to the end user. I mean, it is disabled by default: you have to explicitly indicate version "0.0.0" to enable it, so I don't think it would happen by mistake. But I could add a warning, just in case.

[kaycebasques]

Re: 1 your proposed solution could probably work for us, I'll keep an eye out for your updates!

Re: 2 I did not mean to suggest it was an error on your part. I encountered the same issue in my implementation. It's just a limitation that will need to be worked around somehow. I don't know yet whether it makes more sense to workaround it here in rules_doxygen or in the Pigweed code. I was just mentioning this as an important FYI for us.

Re: 3 sounds good, thanks for considering.

Thanks again for helping here, I very much appreciate your work and am looking forward to hopefully adopting all of this in Pigweed!!

[TendTo]

Ok, I think I managed to pull it off. Let me know if this works for you.
In your case, you should be able to do something like

bazel_dep(name = "rules_doxygen", version = "1.3.0", dev_dependency = True)

doxygen_extension = use_extension("@rules_doxygen//:extensions.bzl", "doxygen_extension")

doxygen_extension.version(
    sha256 = "8354583f86416586d35397c8ee7e719f5aa5804140af83cf7ba39a8c5076bdb8",
    version = "1.9.6",
   platform = "linux"
)
doxygen_extension.version(
    sha256 = "25ad21fa767bd71208947b848d482b46c6bc523a97ba1aacf803c71c4d043726",
    version = "1.9.6",
   platform = "mac"
)

use_repo(doxygen_extension, "doxygen")

[kaycebasques]

Local builds on macOS and Linux looked good! The new MODULE.bazel.lock also looks good. Let me run the CI/CQ tryjobs and see if everything looks OK there. I expect the "unsupported CPU type" issue to still be present for Mac+ARM but we run the tryjobs in a whole bunch of different environments so it's a good way to detect any other unintentional changes in the new code...

[kaycebasques]

Tryjobs all good (except the two that were expected to fail) so I think this is all good to merge as far as I'm concerned! https://pwrev.dev/247192

[TendTo]

Good to hear. If you find a way circumvent the CPU architecture issue let me know and I'll try to implement it in the rule as well.