Subject: new feature for bmake: read-only variables
To: None <tech-pkg@netbsd.org>
From: Roland Illig <rillig@NetBSD.org>
List: tech-pkg
Date: 08/25/2005 12:51:36
This is a multi-part message in MIME format.
--------------050907040208000803020706
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
This patch adds a new feature to bmake. Variables can be marked
read-only, so there is no chance of overwriting them.
PKGNAME= mypackage-1.4
.readonly PKGNAME
This code will work as usual, but as soon as PKGREVISION is defined, it
will blow up in ../../mk/bsd.pkg.mk, line 86. At that point, pkgsrc is
modifying the variable. I consider silent variable modification harmful,
because it makes debugging much harder.
For convenience, there are two more directives, .begin_readonly and
.end_readonly, which can be used like this:
.begin_readonly
TOOLS_PLATFORM.cat?= /bin/cat
TOOLS_PLATFORM.chgrp?= /usr/bin/chgrp
TOOLS_PLATFORM.chmod?= /bin/chmod
TOOLS_PLATFORM.chown?= /usr/sbin/chown
TOOLS_PLATFORM.cmp?= /usr/bin/cmp
TOOLS_PLATFORM.cp?= /bin/cp
TOOLS_PLATFORM.csh?= /bin/csh
TOOLS_PLATFORM.cut?= /usr/bin/cut
TOOLS_PLATFORM.date?= /bin/date
TOOLS_PLATFORM.diff?= /usr/bin/diff
TOOLS_PLATFORM.dirname?= /usr/bin/dirname
TOOLS_PLATFORM.echo?= echo # shell builtin
TOOLS_PLATFORM.egrep?= /usr/bin/egrep
TOOLS_PLATFORM.env?= /usr/bin/env
.end_readonly
I don't expect this feature to be integrated immediately, it's just that
I find it useful for debugging. And, if it would be applied to pkgsrc,
it would make its behavior more predictable.
By the way, when preparing this patch I have rewritten the directive
parser, because the old code (from 1993 and 1994) looked too ugly. The
new parser code is a little stricter, in that the directive .undefined
does not mean "delete the variable ined" anymore.
Roland
--------------050907040208000803020706
Content-Type: text/plain;
name="bmake-readonly-vars.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="bmake-readonly-vars.patch"
? bmake-readonly-vars.patch
Index: nonints.h
===================================================================
RCS file: /cvsroot/pkgsrc/bootstrap/bmake/nonints.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 nonints.h
--- nonints.h 11 Mar 2004 13:04:10 -0000 1.1.1.1
+++ nonints.h 25 Aug 2005 10:32:37 -0000
@@ -152,3 +152,5 @@ char *Var_GetHead __P((char *));
void Var_Init __P((void));
void Var_End __P((void));
void Var_Dump __P((GNode *));
+void Var_Make_Readonly(char *, GNode *);
+void Var_Set_Readonly_Mode(int);
Index: parse.c
===================================================================
RCS file: /cvsroot/pkgsrc/bootstrap/bmake/parse.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 parse.c
--- parse.c 11 Mar 2004 13:04:12 -0000 1.1.1.1
+++ parse.c 25 Aug 2005 10:32:37 -0000
@@ -2534,6 +2534,31 @@ ParseFinishLine()
}
}
+static char *
+skip_word(char *cp)
+{
+ while (*cp != '\0' && !isspace((unsigned char)*cp))
+ cp++;
+ return cp;
+}
+
+static char *
+skip_whitespace(char *cp)
+{
+ while (isspace((unsigned char)*cp))
+ cp++;
+ return cp;
+}
+
+static int
+word_eq(const char *begin, const char *end, const char *word)
+{
+ size_t word_len;
+
+ word_len = strlen(word);
+ return (end - begin == word_len
+ && memcmp(begin, word, end - begin) == 0);
+}
/*-
*---------------------------------------------------------------------
@@ -2568,36 +2593,66 @@ Parse_File(name, stream)
do {
while ((line = ParseReadLine ()) != NULL) {
+
+ /* Check if the line is a directive. */
if (*line == '.') {
+ char *dir, *dir_end, *cont;
+
/*
- * Lines that begin with the special character are either
- * include or undef directives.
+ * Get the name of the directive. The {cont} pointer
+ * tells us where to continue parsing after the directive
+ * has processed its arguments.
*/
- for (cp = line + 1; isspace ((unsigned char)*cp); cp++) {
- continue;
- }
- if (strncmp(cp, "include", 7) == 0 ||
- ((cp[0] == 's' || cp[0] == '-') &&
- strncmp(&cp[1], "include", 7) == 0)) {
- ParseDoInclude (cp);
+ dir = skip_whitespace(line + 1);
+ dir_end = skip_word(dir);
+ cont = dir_end;
+
+ if (word_eq(dir, dir_end, "include") ||
+ word_eq(dir, dir_end, "sinclude") ||
+ word_eq(dir, dir_end, "-include")) {
+ ParseDoInclude (dir);
goto nextLine;
- } else if (strncmp(cp, "undef", 5) == 0) {
- char *cp2;
- for (cp += 5; isspace((unsigned char) *cp); cp++) {
- continue;
- }
- for (cp2 = cp; !isspace((unsigned char) *cp2) &&
- (*cp2 != '\0'); cp2++) {
- continue;
- }
+ } else if (word_eq(dir, dir_end, "undef")) {
+ char *varname;
- *cp2 = '\0';
+ varname = skip_whitespace(dir_end);
+ cont = skip_word(varname);
- Var_Delete(cp, VAR_GLOBAL);
- goto nextLine;
+ Var_Delete(varname, VAR_GLOBAL);
+
+ } else if (word_eq(dir, dir_end, "readonly")) {
+ char *varname;
+
+ varname = skip_whitespace(dir_end);
+ cont = skip_word(varname);
+
+ Var_Make_Readonly(varname, VAR_GLOBAL);
+
+ } else if (word_eq(dir, dir_end, "begin_readonly")) {
+ Var_Set_Readonly_Mode(1);
+
+ } else if (word_eq(dir, dir_end, "end_readonly")) {
+ Var_Set_Readonly_Mode(0);
+
+ } else if (strchr(line, ':') == NULL) {
+ *dir_end = '\0';
+ Parse_Error(PARSE_FATAL, "Unknown directive \"%s\".", dir);
+ /* NOTREACHED */
+ } else {
+ /* Can be something like .SUFFIXES: or .c.o: */
+ goto its_not_a_directive /* below */;
}
+
+ cont = skip_whitespace(cont);
+ if (*cont == '\0' || *cont == '#')
+ goto nextLine;
+
+ Parse_Error(PARSE_FATAL, "Garbage at end of line: %s", cont);
+ /* NOTREACHED */
}
+
+ its_not_a_directive:
if (*line == '#') {
/* If we're this far, the line must be a comment. */
goto nextLine;
Index: var.c
===================================================================
RCS file: /cvsroot/pkgsrc/bootstrap/bmake/var.c,v
retrieving revision 1.2
diff -u -p -r1.2 var.c
--- var.c 23 Aug 2004 03:44:34 -0000 1.2
+++ var.c 25 Aug 2005 10:32:39 -0000
@@ -148,6 +148,11 @@ static char varNoError[] = "";
GNode *VAR_GLOBAL; /* variables from the makefile */
GNode *VAR_CMD; /* variables defined on the command-line */
+/* When non-zero, all assignments mark the destination variable read-only,
+ * so it cannot be modified again.
+ */
+static int var_readonly_mode = 0;
+
#define FIND_CMD 0x1 /* look in VAR_CMD when searching */
#define FIND_GLOBAL 0x2 /* look in VAR_GLOBAL as well */
#define FIND_ENV 0x4 /* look in the environment also */
@@ -166,6 +171,7 @@ typedef struct Var {
#define VAR_KEEP 8 /* Variable is VAR_JUNK, but we found
* a use for it in some modifier and
* the value is therefore valid */
+#define VAR_READONLY 16
} Var;
@@ -388,8 +394,13 @@ VarAdd (name, val, ctxt)
h = Hash_CreateEntry (&ctxt->context, name, NULL);
Hash_SetValue(h, v);
v->name = h->name;
+
+ if (var_readonly_mode)
+ v->flags |= VAR_READONLY;
+
if (DEBUG(VAR)) {
- printf("%s:%s = %s\n", ctxt->name, name, val);
+ printf("%s:%s = %s%s\n", ctxt->name, name, val,
+ (v->flags & VAR_READONLY) ? " (read-only)" : "");
}
}
@@ -421,6 +432,9 @@ Var_Delete(name, ctxt)
register Var *v;
v = (Var *)Hash_GetValue(ln);
+ if (v->flags & VAR_READONLY) {
+ Parse_Error(PARSE_FATAL, "%s: Readonly variables cannot be deleted.", v->name);
+ }
if (v->name != ln->name)
free(v->name);
Hash_DeleteEntry(&ctxt->context, ln);
@@ -429,6 +443,25 @@ Var_Delete(name, ctxt)
}
}
+void
+Var_Make_Readonly(char *name, GNode *ctxt)
+{
+ Hash_Entry *ln;
+ Var *v;
+
+ if (DEBUG(VAR)) {
+ printf("%s:set_readonly %s\n", ctxt->name, name);
+ }
+
+ ln = Hash_FindEntry(&ctxt->context, name);
+ if (ln == NULL) {
+ Parse_Error(PARSE_WARNING, ".readonly on undefined variables has no effect.");
+ } else {
+ v = (Var *)Hash_GetValue(ln);
+ v->flags |= VAR_READONLY;
+ }
+}
+
/*-
*-----------------------------------------------------------------------
* Var_Set --
@@ -472,12 +505,18 @@ Var_Set (name, val, ctxt, flags)
v = VarFind (name, ctxt, 0);
if (v == (Var *) NIL) {
VarAdd (name, val, ctxt);
+ } else if (v->flags & VAR_READONLY) {
+ Parse_Error(PARSE_FATAL, "%s: Readonly variables cannot be modified.", v->name);
} else {
Buf_Discard(v->val, Buf_Size(v->val));
Buf_AddBytes(v->val, strlen(val), (Byte *)val);
+ if (var_readonly_mode)
+ v->flags |= VAR_READONLY;
+
if (DEBUG(VAR)) {
- printf("%s:%s = %s\n", ctxt->name, name, val);
+ printf("%s:%s = %s%s\n", ctxt->name, name, val,
+ (v->flags & VAR_READONLY) ? " (read-only)" : "");
}
}
/*
@@ -535,13 +574,19 @@ Var_Append (name, val, ctxt)
if (v == (Var *) NIL) {
VarAdd (name, val, ctxt);
+ } else if (v->flags & VAR_READONLY) {
+ Parse_Error(PARSE_FATAL, "%s: Readonly variables cannot be modified.", v->name);
} else {
Buf_AddByte(v->val, (Byte)' ');
Buf_AddBytes(v->val, strlen(val), (Byte *)val);
+ if (var_readonly_mode)
+ v->flags |= VAR_READONLY;
+
if (DEBUG(VAR)) {
- printf("%s:%s = %s\n", ctxt->name, name,
- (char *) Buf_GetAll(v->val, (int *)NULL));
+ printf("%s:%s = %s%s\n", ctxt->name, name,
+ (char *) Buf_GetAll(v->val, (int *)NULL),
+ (v->flags & VAR_READONLY) ? " (read-only)" : "");
}
if (v->flags & VAR_FROM_ENV) {
@@ -2926,3 +2971,9 @@ Var_Dump (ctxt)
VarPrintVar(Hash_GetValue(h));
}
}
+
+void
+Var_Set_Readonly_Mode(int onoff)
+{
+ var_readonly_mode = onoff;
+}
--------------050907040208000803020706--