6 ms·
I like the idea behind this but I wouldn’t even put this in a small project. I don’t want to break functionality just because.
by bogota 5y ago
I like the idea behind this but I wouldn’t even put this in a small project. I don’t want to break functionality just because.
- phire 5y agoIt's a compile error, not a runtime error. Though there are still risks it might suddenly hinder you from releasing a build in an emergency.
- mlyle 5y agoIt screws up reproducibility--- stuff that built in the past no longer builds when an external condition changes. I get what this is trying to do, but there's got to be a better mechanism to let your developers know that e.g. a new feature exists upstream than suddenly breaking the build.
- jrockway 5y agoIt shouldn't screw up reproducibility, but rather exposes what kind of unexpected state can leak in. If your build system knows what time it is and that can affect whether or not it compiles, that's a reproducibility problem.
- mlyle 5y ago> If your build system knows what time it is and that can affect whether or not it compiles, that's a reproducibility problem. The very first example is of this: // trigger a compile error if we're past a certain date todo_or_die::after_date!(3000, 1, 1); // its the year 3000! And the second and third examples (this is all of them) are explicitly relying on non-versioned, external state leaking in, too. // or a GitHub issue has closed todo_or_die::issue_closed!("rust-lang", "rust", 44265); // GATs are here! // or the latest version of a crate matches some expression todo_or_die::crates_io!("serde", ">1.0.9000"); // its over 9000!
- iudqnolq 5y agoBut the issue is that the time your build process observes changes, not that you add a place that observes the time. If you're not fakeing time already it'll end up in all sorts of other places.
- jrockway 5y agoYup, that's exactly what I was hinting at. This sort of statement makes it clear that there is an undesired connection to the system clock, but of course all those tests that use time.Now() or whatever are equally likely to become faulty.
- lultimouomo 5y agoI don't think it's accurate to say that it screws up build reproducibility. Rather, it shows that your build is already not-reproducible. Take the time example: if your build has access to the actual clock time, your build is unreproducible, period. For reproducibile build you need a constant, injected clock time. So I'd rather day that if you have reproducibile builds this macros won't work at all.
- ratorx 5y agoI don’t think that’s true (being pedantic). If your build doesn’t depend on the value of the time in any way, then it’s reproducible (assuming it doesn’t use/doesn’t have other irreproducible inputs), even without passing in an injected time. The value of things which don’t affect the output don’t matter for reproducibility. But it’s a one-way implication. If your build DOES depend on the time, it can be made reproducible by setting it to a constant fixed value.
- lultimouomo 5y agoYou're technically correct (which is the best kind of correct!). But I would opine that, if your build system is not hermetic, i.e. you don't inject time and don't prevent access to the external world, you cannot know whether your build is reproducible or not.
- alpaca128 5y agoIt lets you skip all those checks with an environment variable. So if necessary it'll still build. Although I'd prefer having this throw warnings instead of errors at least locally.
- DSingularity 5y agoPR: remove todo