Subject: Re: new feature for bmake: read-only variables
To: None <tech-pkg@netbsd.org>
From: Roland Illig <rillig@NetBSD.org>
List: tech-pkg
Date: 09/01/2005 21:58:06
This is a multi-part message in MIME format.
--------------040508000806080608020404
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Joerg Sonnenberger wrote:
> On Thu, Aug 25, 2005 at 12:51:36PM +0200, Roland Illig wrote:
> 
>>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
> 
> 
> There was a consensus between DragonFly and FreeBSD to not add new
> directives, but use targets instead. E.g. this is better added as
> .READONLY: PKGNAME
> 
> This is the additional benefit of allowing old makes to still work.

That sounds good, so for anyone who wants to see which variables are 
modified deep inside pkgsrc, here's the updated patch. The intended 
usage is:

.BEGIN_READONLY:
VAR= value
.END_READONLY:

VAR2= value2
.READONLY: VAR2

> With regarding to cleaning up the parser, talk with Max Okumoto
> :-)

My patch doesn't need that anymore. But most probably it should be done 
anyway. ;)

Roland

--------------040508000806080608020404
Content-Type: text/plain;
 name="bmake-readonly-vars.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="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	1 Sep 2005 19:52:47 -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 __P((char *, GNode *));
+void Var_Set_Readonly_Mode __P((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	1 Sep 2005 19:52:48 -0000
@@ -164,8 +164,10 @@ Lst         	sysIncPath;	/* list of dire
  */
 typedef enum {
     Begin,  	    /* .BEGIN */
+    Begin_Readonly, /* .BEGIN_READONLY */
     Default,	    /* .DEFAULT */
     End,    	    /* .END */
+    End_Readonly,   /* .END_READONLY */
     Ignore,	    /* .IGNORE */
     Includes,	    /* .INCLUDES */
     Interrupt,	    /* .INTERRUPT */
@@ -186,6 +188,7 @@ typedef enum {
     Posix,	    /* .POSIX */
 #endif
     Precious,	    /* .PRECIOUS */
+    Readonly,       /* .READONLY */
     ExShell,	    /* .SHELL */
     Silent,	    /* .SILENT */
     SingleShell,    /* .SINGLESHELL */
@@ -216,8 +219,10 @@ static struct {
     int	    	  op;	    	/* Operator when used as a source */
 } parseKeywords[] = {
 { ".BEGIN", 	  Begin,    	0 },
+{ ".BEGIN_READONLY", Begin_Readonly, 0 },
 { ".DEFAULT",	  Default,  	0 },
 { ".END",   	  End,	    	0 },
+{ ".END_READONLY", End_Readonly, 0 },
 { ".EXEC",	  Attribute,   	OP_EXEC },
 { ".IGNORE",	  Ignore,   	OP_IGNORE },
 { ".INCLUDES",	  Includes, 	0 },
@@ -244,6 +249,7 @@ static struct {
 { ".POSIX",	  Posix,	0 },
 #endif
 { ".PRECIOUS",	  Precious, 	OP_PRECIOUS },
+{ ".READONLY",    Readonly,     0 },
 { ".RECURSIVE",	  Attribute,	OP_MAKE },
 { ".SHELL", 	  ExShell,    	0 },
 { ".SILENT",	  Silent,   	OP_SILENT },
@@ -1145,6 +1151,12 @@ ParseDoDependency (line)
      */
     if (!*line) {
 	switch (specType) {
+	    case Begin_Readonly:
+		Var_Set_Readonly_Mode(1);
+		break;
+	    case End_Readonly:
+		Var_Set_Readonly_Mode(0);
+		break;
 	    case Suffixes:
 		Suff_ClearSuffixes ();
 		break;
@@ -1191,7 +1203,7 @@ ParseDoDependency (line)
      */
     if ((specType == Suffixes) || (specType == ExPath) ||
 	(specType == Includes) || (specType == Libs) ||
-	(specType == Null))
+	(specType == Null) || (specType == Readonly))
     {
 	while (*line) {
 	    /*
@@ -1240,6 +1252,9 @@ ParseDoDependency (line)
 		case Null:
 		    Suff_SetNull (line);
 		    break;
+		case Readonly:
+		    Var_Make_Readonly (line, VAR_GLOBAL);
+		    break;
 		default:
 		    break;
 	    }
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	1 Sep 2005 19:52:49 -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:make_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;
+}

--------------040508000806080608020404--