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

Package srcs only integrated if under an 'R/' subdirectory #34

Closed
tedks opened this issue Sep 19, 2019 · 8 comments
Closed

Package srcs only integrated if under an 'R/' subdirectory #34

tedks opened this issue Sep 19, 2019 · 8 comments

Comments

@tedks
Copy link

tedks commented Sep 19, 2019

It seems a package will only be properly created from files that are under an 'R' subdirectory.

This seems to go against the notion of Bazel; is there no way to enable a more flexible source structure?

@siddharthab
Copy link
Collaborator

Internally, bazel uses the R build tools to actually compile the package. The srcs attribute merely says which files are available for the build step, but they have to be pre-organized to be compatible with R CMD BUILD.

Categorizing and reorganizing the files is out of scope for these rules, so the user is responsible for following the instructions in https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Package-structure.

@tedks
Copy link
Author

tedks commented Sep 19, 2019

Right. I want to use Bazel to build my R code, specifically so that I don't have to be compatible with another build tool. They are already categorized, as source files. I don't see how putting the fileset under an R subdirectory in the sandbox is "out of scope" for Bazel support for R.

If the intended behavior for this package is to force the R packaging structure on users, it would be helpful for the rules themselves to emit an error when they can tell the R packaging structure is not adhered to. So this is either a bug in that the behavior is the way it is, or a bug in the usability of the rules.

@siddharthab
Copy link
Collaborator

If you don't have an R package, then maybe you are looking for the r_binary rule. You can also chain them together if your R script sources other scripts.

For packages, reorganization is out of scope because it will be fairly complex to keep in sync with the R build toolchain. There are at least 20 different types of files in a package (DESCRIPTION, NAMESPACE, R source code, C source code, manuals, vignettes, bundled data, bundled executables, etc.). Reorganizing all of them will mean that we define a separate attribute for each one of them in the rule, and then hope that we are doing things correctly.

@tedks
Copy link
Author

tedks commented Sep 19, 2019

You're right that I don't have or really want an R package. I want the equivalent of a cc_library rule. The r_library rule looked like the wrong thing to use for this, because of the focus on installation -- maybe I was wrong here, and it would be great if I could switch to that.

I think there are definitely Bazel rules with more than 20 attributes, and that there are probably many more packages that lack most of those things, so it doesn't seem like a very good reason.

Is it intentional that users should be able to build a package without any source code known to be visible to R? This is what you get if you don't have an R subdirectory.

@siddharthab
Copy link
Collaborator

I understand your use better now. What you are looking for are the equivalents of sh_{library|binary|test} rules.

So let's forget about the r_package and r_library rules because they are geared towards package developers. A "library" in R world is a term used for collection of packages.

When I designed the r_{binary|test} rules, I designed them to be almost analogous to sh_{binary|test}. I thought about having the equivalent of sh_library, but decided that the distinction between sh_library and sh_binary was not quite clear anyway and is merely logical (i.e. a script can be both sourced or be run as a standalone), so I collapsed both uses into r_binary.

Please look into the documentation for r_binary, and let me know if things are confusing anywhere. I can improve the documentation or the interface for that.

@tedks
Copy link
Author

tedks commented Sep 20, 2019

It is not really best practice to have things depending on a _binary rule. The distinction is merely logical, but the distinction is a matter of intent. I'm not sure of this, but I think there are linters that will flag this as a problem. It's certainly a code smell.

Nevertheless, thanks for this workaround; it works fine for now.

It would be nice to have an r_library that was analogous to an sh_library, since the existing r_library is more of an r_package_library, as you describe it.

@siddharthab
Copy link
Collaborator

Thanks for the feedback. I can certainly introduce a rule for a logical collection of .R files. Perhaps it will just be a filegroup like rule because I don't really know what else I can do there. Any suggestions on what attributes you would like to see in this rule?

Unfortunately, the name r_library is already taken, so we will have to come up with a name for this as well.

@siddharthab
Copy link
Collaborator

Closing because of inactivity.

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