NFD » ndn-tools

I looked at NFD version processing in wscript.

The code there isn't clean enough: there are too many details that unnecessarily inflates the main wscript.

I plan to:

1. write a bash script to
   1. derive version number from git describe, VERSION file, or hard-coded
   2. save version to VERSION file
2. declare a Waf Task to invoke the bash script, and then subst the output into the code file containing version number

Another issue with NFD version processing is: the version number is declared as a preprocessor macro in a header included by every program.

In an incremental build (on developer workstation), this requires re-compiling every program object whenever version changes.

I plan to put the macro in a .cpp, so that a version change only requires re-linking of every program, and slightly saves compilation time.

The public API would be:

namespace ndn_tools_version {

const std::string BUILD;

void
print(const std::string& programName);

} // namespace ndn_tools_version

When a program sees '-V' command line option, it should call ndn_tools_version::print("ndnping") which prints to stdout:

ndnping 0.1.0

I disagree with note-2. There is no reason in introducing additional script to generate version, if python (wscript) can do it as easy.

For note-3, I would simply define method to get std::string that contains a version number. "print" function is not general enough:

namespace ndn {
namespace tools {

std::string
getVersion();

} // namespace tools
} // namespace ndn


...

std::cout << "programName " << tools::getVersion() << std::endl;

I disagree with note-2. There is no reason in introducing additional script to generate version, if python (wscript) can do it as easy.

The Python code to collect version number has 50 lines. A bash script can do the same in 20 lines or less.

That script had a few different considerations, besides just generating VERSION from git. And I guess it can be simplified, if necessary. Having dependency on python is enough and I don't want to introduce another (unnecessary) dependency on bash/sh.

Alex Afanasyev wrote:

For note-3, I would simply define method to get std::string that contains a version number. "print" function is not general enough:

A function that returns the version string seems overkill to me.

A pattern I've seen used elsewhere is to declare an extern constant in the header and define the value in the cpp. Yet another way is a constexpr function that returns a char const * const, but I'm not sure you can define it out-of-line (and defining it inline kinda defeats the purpose).

note-3 suggested using a constant. @Alex please explain why you want a function instead of a constant.

Constants (if it is not simple char*) may have issues with initialization. At least in the past, I had problems with them (order static constants are initialized/deleted is undetermined).

A function serves the same goal. If use of extern constant is safe, then I have no strong opinion on which way to go.

Alex Afanasyev wrote:

Constants (if it is not simple char*) may have issues with initialization. At least in the past, I had problems with them (order static constants are initialized/deleted is undetermined).

That's the so called "static init order fiasco". I'm proposing extern linkage here, not static. Moreover, this constant can be a simple char const * const, no need for a std::string.

commit:7bab1f75e168a84b17d6ce5fd6ea4a4e48a6b15a declares extern const std::string VERSION.

I disagree with char const* const because a string is semantically correct, and there's no reason to use a C type.

Junxiao Shi wrote:

I disagree with char const* const because a string is semantically correct, and there's no reason to use a C type.

Uh? so we cannot use char or int or double because they're "C types" now?

I prefer to use std::string because it's semantically correct.
If you insist on const char[], please give a technical reason.

I won't change without being convinced with a valid reason.

An std::string requires dynamic initialization, i.e. code is executed before running the main() function every time the program is loaded. A simple const char[] is statically initialized and exists as a constant in the ELF file (.rodata section IIRC) therefore it doesn't require any code to be executed before main().

I'll test whether it's constructed even when not used. If so, I could change.

Also, what's wrong with const std::string& getVersion()?
Using a function is also semantically correct.
Simply "overkill" is not a valid reason.

I don't know what you mean by semantically correct. Keeping things as simple as they can be is a very valid reason in my opinion.

Btw, you haven't mentioned a single drawback of const char[] so far, so it seems to me that it's the best approach from a purely technical point of view.