env: create new package and introduce build variables.

[MSevey]

Proposal

I would like to introduce the concept of build time variables. These build time variables allow for switching between values based on the build, i.e. testing vs production.

Why?

One major benefit of build specific variables is in testing. Timeouts are a common variable to define, and the expectations of timeouts in testing and in production are usually quite different. For example on production, we might measure response time in seconds while in a virtualized test environment we measure in milliseconds.

Ultimately this adds a more granular level of control over our variables.

PR Context

It would have been nice for the new package to be called build since these are build time variables, but we currently use the build directory as the output for make build. I would be open to refactoring that in a follow up if others agree with the naming.

To properly using the build variables, the -tags flag needs to be used in testing (see updates to the Makefile).

I updated one variable in the fraud package to illustrate how this new variable interface can be used.

[codecov-commenter]

Codecov Report

Merging #1280 (bd63366) into main (7dddcfc) will increase coverage by 0.03%.
The diff coverage is 73.33%.

@@            Coverage Diff             @@
##             main    #1280      +/-   ##
==========================================
+ Coverage   56.14%   56.18%   +0.03%     
==========================================
  Files         161      161              
  Lines        9912     9950      +38     
==========================================
+ Hits         5565     5590      +25     
- Misses       3793     3806      +13     
  Partials      554      554              

Impacted Files	Coverage Δ
fraud/requester.go	48.14% <ø> (ø)
env/var.go	73.33% <73.33%> (ø)
nodebuilder/p2p/flags.go	48.68% <0.00%> (-10.58%)	⬇️
cmd/celestia/full.go	36.76% <0.00%> (-3.56%)	⬇️
cmd/celestia/light.go	36.23% <0.00%> (-3.46%)	⬇️
nodebuilder/settings.go	0.00% <0.00%> (-3.34%)	⬇️
cmd/celestia/bridge.go	74.13% <0.00%> (-2.79%)	⬇️
share/ipld/get.go	91.20% <0.00%> (-1.39%)	⬇️
cmd/flags_node.go	60.46% <0.00%> (-0.94%)	⬇️
... and 21 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[evan-forbes]

what's the status of this? is this meant to be used org wide? if so, should we move it somewhere else?

[Wondertan]

I am very much in favor of such functionality. I believe that some binary configurations should not be exposed via user-facing configs. Users tend to play around with things, break them, report and then waste their time debugging weird edge cases that wouldn't happen if we just hide them. Such things should be hidden from them via compile-time flags, like in this PR.

Requesting changes to remove fraud writeDeadline example for a few reasons:

• Chozen writeDeadline is something we already planned to tackle via Cleanup global variables #709 and wanted to follow the das: make DAS'er params to be configurable  #1063 example. So we essentially want to remove it from consts.
  • I wouldn't say that the writeDeadline needs production/testing configurations, but I understand that this is just an example you wanted to show us, which I appreciate.
• The introduced env package brings celestia-node application build semantics to it. We usually aim to avoid this and keep our root-level pkgs as independent as possible. The main reason is the reusability of pkgs by other teams or even other projects. This is now happening to header pkg, and rollmint team will use it for all things headers instead of reinventing the wheel. Making header dependent on env pkg would require them to use it as well when they might not need it. The same thing will happen to the fraud pkg.

Happy to merge it the way it is without any examples, but if we still want one, let's try it for #63, which could be a good example of a build time var. The integration should work with p2p module which is currently located in nodebuilder/p2p

[MSevey]

@Wondertan I can remove the example, no problem.

As for the package level concern, maybe we can address that with @evan-forbes question. Is there maybe a better org level repo to put this in?

We could even create a utils repo that has things like this and other common helpers that many different repos could use.