Move the generated version from a .h to a .c file

Reinstate commit 4a438e5 that was reverted by commit 91b3806. That commit reversion was a mistake because it appeared, based on the problem description, that the change incorrectly constrained the valid version strings. Subsequent analysis of the problem showed that not to be the case and the problem reporter was, in fact, attempting to use a non-semantic version string. The SH_VERSION preprocessor symbol has bothered me for a long time. Especially as used in the context of `e_version[]`. This simplifies how the version is injected by the build process into the code; notwithstanding the large number of files touched by this change.
att · Nov 15, 2019 · d72c3a6 · d72c3a6
1 parent 7c92227
commit d72c3a6
Show file tree

Hide file tree

Showing 19 changed files with 43 additions and 132 deletions.
diff --git a/config_ast.h.in b/config_ast.h.in
@@ -225,6 +225,7 @@ typedef void (*sighandler_t)(int);
 
 // Types that are needed in many places but which are difficult to define in the header containing
 // the structure definition due to circular includes that would otherwise result.
+typedef struct Namfun Namfun_t;      // from name.h
 typedef struct Namval Namval_t;      // from name.h
 typedef struct Shell_s Shell_t;      // from shell.h
 typedef struct Shbltin_s Shbltin_t;  // from shcmd.h

diff --git a/scripts/builtin_documentation.sh b/scripts/builtin_documentation.sh
@@ -3,8 +3,6 @@
 # the raw markup text.
 out_file="$MESON_BUILD_ROOT/$MESON_SUBDIR/documentation.c"
 exec > "$out_file"
-# `ksh_mini.1` uses `SH_VERSION` macro to generate version number string
-echo '#include "version.h"'
 for in_file in "$MESON_SOURCE_ROOT/src/cmd/ksh93/docs/"*.1
 do
     cmd_name=$(basename "$in_file" .1)

diff --git a/src/cmd/ksh93/data/variables.c b/src/cmd/ksh93/data/variables.c
@@ -26,7 +26,6 @@
 #include "defs.h"
 #include "name.h"
 #include "shtable.h"
-#include "variables.h"  // IWYU pragma: keep
 
 //
 // This is the list of built-in shell variables and default values and default attributes.
@@ -58,7 +57,7 @@ const struct shtable2 shtab_variables[] = {
     {".sh.subscript", 0, NULL},
     {".sh.subshell", NV_INTEGER | NV_SHORT | NV_NOFREE, NULL},
     {".sh.value", 0, NULL},
-    {".sh.version", NV_NOFREE, (char *)(&e_version[10])},
+    {".sh.version", NV_NOFREE, NULL},
     {"CDPATH", 0, NULL},
     {"COLUMNS", 0, NULL},
     {"COMPREPLY", 0, NULL},

diff --git a/src/cmd/ksh93/docs/ksh_mini.1 b/src/cmd/ksh93/docs/ksh_mini.1
@@ -1,5 +1,5 @@
 +[-1?
-@(#)$Id: sh (AT&T Research) " SH_RELEASE " $
+@(#)$Id: sh (AT&T Research) 2020.0.0 $
 ]
 [+NAME?\b\f?\f\b - Shell, the standard command language interpreter]
 [+DESCRIPTION?\b\f?\f\b is a command language interpreter that executes commands read from a command line string, the standard input, or a specified file.]

diff --git a/src/cmd/ksh93/edit/emacs.c b/src/cmd/ksh93/edit/emacs.c
@@ -253,7 +253,7 @@ int ed_emacsread(void *context, int fd, char *buff, int scend, int reedit) {
                 goto do_default_processing;
             }
             case cntl('V'): {
-                emacs_show_info(ep, fmtident(e_version));
+                emacs_show_info(ep, e_version);
                 continue;
             }
             case '\0': {
@@ -662,7 +662,7 @@ static_fn int emacs_escape(Emacs_t *ep, wchar_t *out, int count) {
     if (value < 0) value = 1;
     switch (ch = i) {
         case cntl('V'): {
-            emacs_show_info(ep, fmtident(e_version));
+            emacs_show_info(ep, e_version);
             return -1;
         }
         case ' ': {

diff --git a/src/cmd/ksh93/edit/vi.c b/src/cmd/ksh93/edit/vi.c
@@ -422,10 +422,12 @@ static_fn int cntlmode(Vi_t *vp) {
                 break;
             }
             case cntl('V'): {
-                const char *p = fmtident(e_version);
                 vi_save_v(vp);
                 vi_del_line(vp, BAD);
+
+                const char *p = e_version;
                 while ((c = *p++)) append(vp, c, APPEND);
+
                 vi_refresh(vp, CONTROL);
                 ed_getchar(vp->ed, -1);
                 vi_restore_v(vp);

diff --git a/src/cmd/ksh93/include/meson.build b/src/cmd/ksh93/include/meson.build
diff --git a/src/cmd/ksh93/include/name.h b/src/cmd/ksh93/include/name.h
@@ -201,7 +201,6 @@ struct Value {
 #define IS_VT(value_obj, which) is_vt(value_obj, which)
 #define IS_VTP(value_objp, which) is_vtp(value_objp, which)
 
-typedef struct Namfun Namfun_t;
 typedef struct Namdisc Namdisc_t;
 typedef struct Nambfun Nambfun_t;
 typedef struct Namarray Namarr_t;

diff --git a/src/cmd/ksh93/include/shcmd.h b/src/cmd/ksh93/include/shcmd.h
@@ -25,6 +25,8 @@
 #ifndef _SHCMD_H
 #define _SHCMD_H 1
 
+#include "sfio.h"
+
 #define SH_PLUGIN_VERSION 20111111L
 
 // #define SHLIB(m)
@@ -54,5 +56,6 @@ struct Shbltin_s {
 #define bltin_checksig(c) (c && c->sigset)
 
 extern int cmdinit(int, char **, Shbltin_t *, int);
+extern Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp);
 
 #endif  // _SHCMD_H
diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h
@@ -87,7 +87,8 @@ extern const char e_format[];
 extern const char e_number[];
 extern const char e_restricted[];
 extern const char e_recursive[];
-extern char e_version[];
+extern const char *e_version;
+extern const char *ksh_version;
 
 typedef struct sh_scope {
     struct sh_scope *par_scope;

diff --git a/src/cmd/ksh93/include/version.h.in b/src/cmd/ksh93/include/version.h.in
diff --git a/src/cmd/ksh93/meson.build b/src/cmd/ksh93/meson.build
@@ -5,7 +5,6 @@ ksh93_incdir = include_directories('include',
                                    '../../lib/libast/include/',
                                    '../../lib/libdll/')
 
-subdir('include')
 subdir('bltins')
 subdir('cmds')
 subdir('data')

diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
@@ -61,25 +61,11 @@
 #include "shtable.h"
 #include "stk.h"
 #include "variables.h"
-#include "version.h"
 
 #if USE_SPAWN
 #include "spawnvex.h"
 #endif
 
-char e_version[] =
-    "\n@(#)$Id: Version "
-#define ATTRS 1
-    "A"
-#if SHOPT_BASH
-#define ATTRS 1
-    "B"
-#endif
-#if ATTRS
-    " "
-#endif
-    SH_RELEASE " $\0\n";
-
 #if SHOPT_BASH
 extern void bash_init(Shell_t *, int);
 #endif
@@ -837,52 +823,45 @@ static_fn char *name_match(const Namval_t *np, Namfun_t *fp) {
 static const Namdisc_t SH_MATCH_disc = {
     .dsize = sizeof(struct match), .getval = get_match, .namef = name_match};
 
-static_fn char *get_version(Namval_t *np, Namfun_t *fp) { return nv_getv(np, fp); }
+static_fn char *get_version(Namval_t *np, Namfun_t *fp) {
+    UNUSED(np);
+    UNUSED(fp);
+
+    return (char *)ksh_version;  // discard const qualifier -- we expect the caller to not mutate
+}
 
 // This is invoked when var `.sh.version` is used in a numeric context such as
 // `$(( .sh.version ))`.
-static_fn Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) {
+Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) {
     UNUSED(np);
+    UNUSED(fp);
 
     // We should not need to convert version number string every time this function is called
     static Sflong_t version_number = -1;
     if (version_number != -1) return (Sfdouble_t)version_number;
 
-    char *cp = strdup(SH_RELEASE);
-    char *dash;
     char *dot;
-    char *major_str, *minor_str, *patch_str;
+    const char *major_str, *minor_str, *patch_str;
     int major, minor, patch;
-    UNUSED(fp);
-
-    // Version string in development version could be set like 2017.0.0-devel-1509-g95d59865
-    // If a '-' exists in version string, set it as end of string i.e. version string becomes
-    // 2017.0.0
-    dash = strchr(cp, '-');
-    if (dash) *dash = 0;
 
     // Major version number starts at beginning of string
-    major_str = cp;
+    major_str = ksh_version;
 
     // Find the first '.' and set it to NULL, so major version string is set to 2017
-    dot = strchr(cp, '.');
+    dot = strchr(ksh_version, '.');
     if (!dot) {
         // If there is no . in version string, it means version string is either empty, invalid
         // or it's using old versioning scheme.
-        free(cp);
         version_number = 0;
         return version_number;
     }
 
-    *dot = 0;
-
     // Minor version string starts after first '.'
     minor_str = dot + 1;
 
     // Find the second '.' and set it to NULL, so minor version string is set to 0
     dot = strchr(minor_str, '.');
     assert(dot);
-    *dot = 0;
 
     // Patch number starts after second '.'
     patch_str = dot + 1;
@@ -893,10 +872,9 @@ static_fn Sfdouble_t nget_version(Namval_t *np, Namfun_t *fp) {
 
     assert(minor < 100);
     assert(patch < 100);
-    // This will break if minor or patch number goes above 99
+    // This will break if minor or patch number goes above 99. Hence the preceding assert()'s.
     version_number = major * 10000 + minor * 100 + patch;
 
-    free(cp);
     return (Sfdouble_t)version_number;
 }
 
@@ -1092,8 +1070,6 @@ Shell_t *sh_init(int argc, char *argv[], Shinit_f userinit) {
     int type;
     static char *login_files[2];
 
-    n = strlen(e_version);
-    if (e_version[n - 1] == '$' && e_version[n - 2] == ' ') e_version[n - 2] = 0;
     if (!beenhere) {
         beenhere = 1;
         shp = sh_getinterp();
@@ -1273,7 +1249,7 @@ Shell_t *sh_init(int argc, char *argv[], Shinit_f userinit) {
     sh_offstate(shp, SH_INIT);
     login_files[0] = (char *)e_profile;
     shp->gd->login_files = login_files;
-    shp->bltindata.version = SH_VERSION;
+    shp->bltindata.version = nget_version(NULL, NULL);
     shp->bltindata.shp = shp;
     shp->bltindata.shrun = sh_run;
     shp->bltindata.shexit = sh_exit;

diff --git a/src/cmd/ksh93/sh/meson.build b/src/cmd/ksh93/sh/meson.build
@@ -1,4 +1,9 @@
+vcs_version = vcs_tag(command: ['git', 'describe', '--always', '--dirty', '--tags'],
+                      input: 'version.c.in', output: 'version.c',
+                      fallback: get_option('fallback-version-number'))
+
 ksh93_files += [
+    vcs_version,
     'sh/args.c',
     'sh/arith.c',
     'sh/array.c',

diff --git a/src/cmd/ksh93/sh/version.c.in b/src/cmd/ksh93/sh/version.c.in
@@ -0,0 +1,12 @@
+// This is used as input to the Meson vcs_tag() function to produce the corresponding version.c
+// module.
+#include "config_ast.h"  // IWYU pragma: keep
+
+const char *ksh_version = "@VCS_TAG@";
+const char *e_version =
+    "Version "
+    "A"
+#if SHOPT_BASH
+    "B"
+#endif
+    " @VCS_TAG@";
diff --git a/src/cmd/ksh93/tests/coprocess.sh b/src/cmd/ksh93/tests/coprocess.sh
@@ -263,7 +263,6 @@ done
 exp=ksh
 got=$(print -r $'#00315
 COATTRIBUTES=\'label=make \'
-# @(#)$Id: libcoshell (AT&T Research) 2008-04-28 $
 _COSHELL_msgfd=5
 { { (eval \'function fun { trap \":\" 0; return 1; }; trap \"exit 0\" 0; fun; exit 1\') && PATH= print -u$_COSHELL_msgfd ksh; } || { times && echo bsh >&$_COSHELL_msgfd; } || { echo osh >&$_COSHELL_msgfd; }; } >/dev/null 2>&1' | $SHELL 5>&1)
 [[ $got == $exp ]] || log_error "coshell(3) identification sequence failed -- expected '$exp', got '$got'"

diff --git a/src/lib/libast/include/ast.h b/src/lib/libast/include/ast.h
@@ -152,7 +152,6 @@ extern char *fgetcwd(int, char *, size_t);
 extern char *fmtbase(int64_t, int, int);
 extern char *fmtbuf(size_t);  // to be used only by the "string" subsystem
 extern char *fmtelapsed(unsigned long, int);
-extern char *fmtident(const char *);
 extern char *fmtfmt(const char *);
 extern char *fmtint(int64_t, int);
 extern char *fmtmatch(const char *);

diff --git a/src/lib/libast/string/fmtident.c b/src/lib/libast/string/fmtident.c
diff --git a/src/lib/libast/string/meson.build b/src/lib/libast/string/meson.build
@@ -6,7 +6,6 @@ libast_files += [
     'string/fmtelapsed.c',
     'string/fmtesc.c',
     'string/fmtfmt.c',
-    'string/fmtident.c',
     'string/fmtint.c',
     'string/fmtmatch.c',
     'string/fmtmode.c',