Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate docs for pkg_install #897

Open
jacky8hyf opened this issue Sep 30, 2024 · 6 comments
Open

Generate docs for pkg_install #897

jacky8hyf opened this issue Sep 30, 2024 · 6 comments

Comments

@jacky8hyf
Copy link
Contributor

jacky8hyf commented Sep 30, 2024

@aiuto Are we okay to add pkg_install to the generated docs?

In particular, I think the following needs to be done:

  1. Dependencies of pkg_install (in particular rules_python) needs to be added to @rules_pkg//doc_build:rules_pkg_lib
  2. Add pkg_install to ORDER:
    ORDER = [
    ("toc", None),
    ("common", None),
    ("pkg_deb", "//pkg/private/deb:deb.bzl"),
    ("pkg_deb_impl", "//pkg/private/deb:deb.bzl"),
    ("pkg_sub_rpm", "//pkg:rpm_pfg.bzl"),
    ("pkg_rpm", "//pkg:rpm_pfg.bzl"),
    ("pkg_tar", "//pkg/private/tar:tar.bzl"),
    ("pkg_tar_impl", "//pkg/private/tar:tar.bzl"),
    ("pkg_zip", "//pkg/private/zip:zip.bzl"),
    ("pkg_zip_impl", "//pkg/private/zip:zip.bzl"),
    ("mappings", None),
    ("legacy_pkg_rpm", None),
    ]
@jacky8hyf
Copy link
Contributor Author

On a second thought, I think the dependencies like skylib paths and rules_python needs to be added to @rules_pkg//pkg:bzl_srcs instead. This is because a client cannot directly depend on @rules_pkg//doc_build:rules_pkg_lib. Because of bzlmod, rules_pkg cannot see @stardoc when it is not the root module, because it is a dev dependency.

This also follows how other rules_* handles things (that the bzl_library stays beside the .bzl file), e.g. https://github.com/bazelbuild/rules_python/blob/main/python/BUILD.bazel#L52.

jacky8hyf pushed a commit to jacky8hyf/rules_pkg that referenced this issue Oct 1, 2024
... next to the .bzl files. This is because the
//doc_build package is not loadable when rules_pkg itself
is not the root Bazel module, because stardoc is a
dev_dependency. So, //doc_build:rules_pkg_lib is not
usable by clients of rules_pkg.

Move the target to //pkg, next to the .bzl files.

Also add @rules_python//python:defs_bzl used by pkg_install.

Link: bazelbuild#897
Signed-off-by: HONG Yifan <[email protected]>
jacky8hyf pushed a commit to jacky8hyf/rules_pkg that referenced this issue Oct 1, 2024
... next to the .bzl files. This is because the
//doc_build package is not loadable when rules_pkg itself
is not the root Bazel module, because stardoc is a
dev_dependency. So, //doc_build:rules_pkg_lib is not
usable by clients of rules_pkg.

Move the target to //pkg, next to the .bzl files.

Also add @rules_python//python:defs_bzl used by pkg_install.

Link: bazelbuild#897
Signed-off-by: HONG Yifan <[email protected]>
jacky8hyf pushed a commit to jacky8hyf/rules_pkg that referenced this issue Oct 1, 2024
... next to the .bzl files. This is because the
//doc_build package is not loadable when rules_pkg itself
is not the root Bazel module, because stardoc is a
dev_dependency. So, //doc_build:rules_pkg_lib is not
usable by clients of rules_pkg.

Move the target to //pkg, next to the .bzl files.

Also add @rules_python//python:defs_bzl used by pkg_install.

Link: bazelbuild#897
Signed-off-by: HONG Yifan <[email protected]>
@aiuto
Copy link
Collaborator

aiuto commented Oct 2, 2024

What is wrong with adding pkg_install like all the other docs.
The hackery for doc_build is there because straightforward stardoc didn't the features I wanted.
I was also trying not to take a non-dev dependency on skylib because skylib also depends on rules_pkg for building a distribution.
I've learn (the hard way) that when we have dependency loops, it is incredibly difficult to every do a release that bumps compatibility level, because bzlmod gets confused.

@jacky8hyf
Copy link
Contributor Author

jacky8hyf commented Oct 3, 2024

(copy pasted from off-thread chat)

I am not sure what you mean by your comment.

What I need is one bzl_library that contains install.bzl & @rules_python//python:defs_bzl. As a client, I can't put @rules_pkg//pkg:bzl_srcs to my stardoc() rule, because it is missing @rules_python//python:defs_bzl.

ERROR: /mnt/sdc/android/kernel/build/kernel/kleaf/docs/BUILD.bazel:66:12: in deps attribute of starlark_doc_extract rule //build/kernel/kleaf/docs:common_kernels.bzl.extract: missing bzl_library targets for Starlark module(s) @@rules_python~//python:defs.bzl. Since this rule was created by the macro 'stardoc', the error might have been caused by the macro implementation

I could add @rules_python//python:defs_bzl to my stardoc(), but that requires me to add rules_python to my MODULE.bazel. But in fact, this is a transitive dependency, so my module should not add rules_python directly. Instead, the bzl_library that provides install.bzl (@rules_pkg//pkg:bzl_srcs in this case) should give me this implicit dependency.

The //doc_build:rules_pkg_lib is another candidate, but it can't be used when rules_pkg itself is not the root module (as I explained in the pull request). The error is the following:

ERROR: error loading package '@@rules_pkg~//doc_build': Unable to find package for @@[unknown repo 'stardoc' requested from @@rules_pkg~]//stardoc:stardoc.bzl: The repository '@@[unknown repo 'stardoc' requested from @@rules_pkg~]' could not be resolved: No repository visible as '@stardoc' from repository '@@rules_pkg~'.
ERROR: /mnt/sdc/android/kernel/build/kernel/kleaf/docs/BUILD.bazel:45:12: error loading package '@@rules_pkg~//doc_build': Unable to find package for @@[unknown repo 'stardoc' requested from @@rules_pkg~]//stardoc:stardoc.bzl: The repository '@@[unknown repo 'stardoc' requested from @@rules_pkg~]' could not be resolved: No repository visible as '@stardoc' from repository '@@rules_pkg~'. and referenced by '//build/kernel/kleaf/docs:deps'

This is because stardoc is a dev dependency of rules_pkg. Which, I think, is WAI; it does not make sense to make stardoc a non-dev dependency. A lot of ther modules do the exact same thing as well.

So I could add deps to //pkg:bzl_srcs, or expose //doc_build:rules_pkg_lib; I just take the latter approach, because the name bzl_srcs seems to suggest that this should be just a simple group of srcs without the dependencies. But let me know if you want me to add deps to //pkg:bzl_srcs instead.

@aiuto
Copy link
Collaborator

aiuto commented Oct 8, 2024

What do you mean by "I could add @rules_python//python:defs_bzl to my stardoc(), "
Are you trying to run stardoc on the rules_pkg stuff from another repository?
That is not a use case I think we have any obligation to support cleanly.

@jacky8hyf
Copy link
Contributor Author

What do you mean by "I could add @rules_python//python:defs_bzl to my stardoc(), "
Are you trying to run stardoc on the rules_pkg stuff from another repository?
That is not a use case I think we have any obligation to support cleanly.

No. I am trying to run stardoc on my bzl files, which relies on rules_pkg. For example, the dependency is here:

https://cs.android.com/android/kernel/superproject/+/common-android-mainline:build/kernel/kleaf/common_kernels.bzl;l=20;drc=913c2943377d9888e250d2a0094b221aab831bb7

and I am trying to run stardoc here:

https://cs.android.com/android/kernel/superproject/+/common-android-mainline:build/kernel/kleaf/docs/BUILD.bazel

To generate docs for common_kernels.bzl, my stardoc rule needs to depend on @rules_pkg//pkg:bzl_srcs as well as @rules_python//python:defs_bzl. I temporarily worked around this with http://r.android.com/3293955, but I would expect @rules_pkg//pkg:bzl_srcs to transitively include @rules_python//python:defs_bzl.

@aiuto
Copy link
Collaborator

aiuto commented Oct 14, 2024

Sigh. Not having the StarlaryLibaryInfo as a bazel built in is a constant pain.
I really don't want the non-dev dependency loop between the two repos. A few weeks ago I learned the hard way that
trying to update a compatibility level of one of a pair of mutually dependent repos runs breaks central repository verification tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants