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

Enhancement request: Warn if stamp = True is used with no other input files #4842

Closed
mmorearty opened this issue Mar 14, 2018 · 6 comments
Closed
Labels
category: misc > misc P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Server Issues for serverside rules included with Bazel type: feature request

Comments

@mmorearty
Copy link
Contributor

mmorearty commented Mar 14, 2018

Description of the problem / feature request:

It is a feature, not a bug, of build-stamping (genrule stamp = True) that the genrule will only be re-executed if one of the input files other than bazel-out/volatile-status.txt has changed.

This is a good feature; but it is subtle, and seems to be something that a lot of people miss. One example, apparently from within Google: See step 3 under https://github.com/bazelbuild/rules_nodejs/blob/de5393f683b9a73d69d023ca0ffce8ed5d39fcfd/README.md#stamping

Bazel could help with this. If Bazel encounters a genrule which includes stamp = True, but has no srcs at all, it would make sense to omit an error message (or at least a warning; but I think an error is appropriate).

Feature requests: what underlying problem are you trying to solve with this feature?

People mis-using stamp = True, assuming that a genrule with stamp = True and no input files can be used to generate an intermediate output file that can then be used by other targets.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

WORKSPACE: empty file

BUILD:

# main.out will contain main.txt plus bazel-out/volatile-status.txt.
# Since the below rule `mystamp` is incorrectly written, this will always
# end up writing the same volatile-status.txt to main.out even main.in
# changes.
genrule(
    name = "main",
    output_to_bindir = True,
    srcs = ["main.in", "mystamp"],
    outs = ["main.out"],
    cmd = "cat $(SRCS) > $@",
)

# This is an incorrectly written rule, because it has `stamp = True`, but
# no `srcs`, so it will not behave as the author intended. It will only
# run once.
genrule(
    name = "mystamp",
    output_to_bindir = True,
    outs = ["mystamp.txt"],
    stamp = True,
    cmd = "cat bazel-out/volatile-status.txt > $@",
)

main.in:

this is main.txt

To repro the problem:

  • bazel build :main, then look at bazel-bin/main.out
  • Edit main.in
  • bazel build :main again, and then look at the new bazel-bin/main.out
  • Notice that the BUILD_TIMESTAMP line of bazel-bin/main.out has not changed! It would be great if Bazel had given me an error in this case, since I wrote the mystamp rule incorrectly.

What operating system are you running Bazel on?

Ubuntu Linux 14.04

What's the output of bazel info release?

release 0.11.1

Have you found anything relevant by searching the web?

Searched bazelbuild/bazel/issues for is:open stamp

@alexeagle
Copy link
Contributor

@buchgr related, the issue @mmorearty points out in rules_nodejs is because I couldn't find a way to apply a stamp to arbitrary rules (I'd rather have it on my *_package rules).
In my experimentation, stamp only gathers the volatile-status.txt input for genrule and other native rules. Can you suggest an alternative?

@hlopko hlopko added P3 We're not considering working on this, but happy to review a PR. (No assignee) category: misc > misc labels Mar 21, 2018
@ulfjack
Copy link
Contributor

ulfjack commented Mar 21, 2018

@alexeagle - are you saying that there's no way to add stamp inputs for Skylark actions? That's independent of this feature request?

@alexeagle
Copy link
Contributor

@ulfjack I haven't found a way to do it yet.

Also the only resource I could find on stamping has an empty-srcs[] on the genrule too: https://www.kchodorow.com/blog/2017/03/27/stamping-your-builds/

@ulfjack ulfjack removed their assignment Nov 19, 2018
@aiuto aiuto added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website team-Rules-Server Issues for serverside rules included with Bazel and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Aug 23, 2019
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 3 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 28, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: misc > misc P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Server Issues for serverside rules included with Bazel type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants