usnistgov / ndn-dpdk

NDN-DPDK: High-Speed Named Data Networking Forwarder
https://www.nist.gov/publications/ndn-dpdk-ndn-forwarding-100-gbps-commodity-hardware
Other
131 stars 26 forks source link

"More unique" names for env variables #11

Closed Pesa closed 3 years ago

Pesa commented 4 years ago

Many environment variables have very generic names such as MGMT and LOG. Can we rename them to something more project-specific to avoid conflicts? e.g. add an NDNDPDK_ prefix or similar.

yoursunny commented 4 years ago

I plan to eliminate runtime environment variables altogether:

I don't plan to change compile time environment variables, such as RELEASE and BPFCC. Other projects are already using environment variables such as CC instead of BOOST_CC.

Pesa commented 4 years ago

I don't plan to change compile time environment variables, such as RELEASE and BPFCC. Other projects are already using environment variables such as CC instead of BOOST_CC.

I wasn't referring to those (they are standard), with one exception: RELEASE... any reason you can't use the standard NDEBUG? or you want to keep it independent from the behavior of assert?

Pesa commented 4 years ago
  • LOG will be replaced by API. This does not mean log levels can be changed at runtime.

I'm not entirely sure what you mean by this and, depending on what you mean, it may not be a good idea.

The logging subsystem must be simple and reliable and work even when other things are broken, because it may be used for debugging those broken things. Making it depend on too many higher-level components such as an external API could compromise its reliability and therefore its usefulness. Being able to configure logging in a simple way via the environment is often a useful feature when you want to quickly debug something.

yoursunny commented 4 years ago

It turns out that LOG_* has to remain as environment variables, because static initialization occurs before the activate mutation. Thus, I will rename it to NDNDPDK_LOG_*.


NDEBUG is a C macro. It is not an environment variable.

Pesa commented 4 years ago

I only mentioned NDEBUG because you mentioned RELEASE, which is not an env variable either (it's a make variable), and I assumed is turned into a macro of the same name to affect code generation. And the standard macro for that is NDEBUG. Anyway, as I said, I was not talking about compilation.

Pesa commented 4 years ago

RELEASE, which is not an env variable either (it's a make variable)

oh, it looks like the README tells users to prepend RELEASE=1 so yes, that's technically an env variable. It's a bit strange though, variables to make are usually passed on the command line, e.g. make CC=foo RELEASE=1.

yoursunny commented 4 years ago

RELEASE needs to be an environment variable during compile time. It is checked by bash scripts invoked by Makefile. https://github.com/usnistgov/ndn-dpdk/blob/5782ffdcf31b259051ecfc19b33b4c9f35121145/mk/cflags.sh#L5-L7 It is then passed as CFLAGS environment variable for meson and written into cgoflags.go files for Cgo.

@yoursunny has been inventing weird build procedures since 2006, and this isn't the worst...

yoursunny commented 3 years ago

All runtime environment variables have NDNDPDK_ prefix now.