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

Please add a repository_ctx method for reading files by Label #3766

Closed
jmillikin opened this issue Sep 20, 2017 · 9 comments
Closed

Please add a repository_ctx method for reading files by Label #3766

jmillikin opened this issue Sep 20, 2017 · 9 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@jmillikin
Copy link
Contributor

A repository_ctx can have Label attributes that point to files in other repositories (useful for tool plumbing), but such attributes are awkward to work with because there's no direct way to read their content.

The best workaround I've found is to use symlinks and run cat as a subprocess, which certainly deserves an entry in the Useless Use of Cat Award.

def _some_repo_rule(ctx):
  ctx.symlink(ctx.attr.genrule_cmd_file, "external-genrule-cmd.sh")
  cat_genrule_cmd = ctx.execute(["cat", "external-genrule-cmd.sh"])
  if cat_genrule_cmd.return_code != 0:
    fail("Failed to read genrule command %r: %s" % (
        ctx.attr.genrule_cmd_file, cat_genrule_cmd.stderr))
  genrule_content = cat_genrule_cmd.stdout

That whole mess could, ideally, be simplified down to this:

def _some_repo_rule(ctx):
  genrule_content = ctx.read_file(ctx.attr.genrule_cmd_file)
@ittaiz
Copy link
Member

ittaiz commented Sep 20, 2017

This sounds related to (though definitely not a duplicate of) #3438

@hlopko hlopko added category: extensibility > skylark P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request labels Oct 10, 2017
@laurentlb laurentlb added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. and removed category: extensibility > skylark labels Nov 21, 2018
@cgruber
Copy link
Contributor

cgruber commented Jan 18, 2019

Has there been any progress on this, or prioritization? It's worse than the issue description when you consider windows compatibility, as "cat" isn't an external executable on that platform (it being a powershell alias to Get-Content, which is also not an external executable file. So for windows, one has to implement platform-specific toolchains to supply a call to powershell with that command, all to read text file contents with a repository_ctx.

@jmillikin
Copy link
Contributor Author

PR: #7309

@dslomov
Copy link
Contributor

dslomov commented Jan 31, 2019

What is the motivation for reading the file into Starlark rule?

@cgruber
Copy link
Contributor

cgruber commented Jan 31, 2019

Getting metadata in that you can use to generate the repository more effectively. In my case, for example, getting teh appropriate maven metadata (pom files) to ensure that we generate dependencies properly in the generated bazel targets. Right now I'm using "cat" which doesn't work on windows. I don't want to have to set up a per-platform toolchain just to read a file.

@jmillikin
Copy link
Contributor Author

jmillikin commented Jan 31, 2019

I've been reading files in repository rules for two reasons:

  1. Sometimes the choice of which artifacts to download is driven by a file from the main workspace. For example, Python or Node.JS projects save their download URLs to "lock files". The only way to access the content of a file from the main workspace is via the symlink hack described in the first post.

  2. Some external dependencies require running a setup tool, which outputs files with non-deterministic paths. I want to inspect the log file generated by those tools to discover the paths, so they can be inserted into the generated BUILD file.

@jin
Copy link
Member

jin commented Feb 19, 2019

+1, would be nice to have an OS-independent cat like feature in a repository rule.

@jin
Copy link
Member

jin commented Feb 26, 2019

This came up in a discussion with @dkelmer earlier on again. Currently, to do cross-platform cat in a repository rule, we'll need to do this:

def _cat_file(repository_ctx, filepath):
    # For Windows, use cat from msys.
    cat = "C:\\msys64\\usr\\bin\\cat" if (_is_windows(repository_ctx)) else repository_ctx.which("cat")
    exec_result = repository_ctx.execute([cat, repository_ctx.path(filepath)])
    if (exec_result.return_code != 0):
        fail("Error while trying to read %s: %s" % (filepath, exec_result.stderr))
    return exec_result.stdout

It'll be nice to shorten this to content = repository_ctx.read(file).

@cgruber
Copy link
Contributor

cgruber commented Mar 8, 2019

I mean, it's that or add toolchains. :)

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants