diff mbox series

[2/6] fixdep: parse Makefile more correctly to handle comments etc.

Message ID 20221231064203.1623793-3-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series kbuild: fix dep-file processing for rust | expand

Commit Message

Masahiro Yamada Dec. 31, 2022, 6:41 a.m. UTC
fixdep parses dependency files (*.d) emitted by the compiler.

*.d files are Makefiles describing the dependencies of the main source
file.

fixdep understands minimal Makefile syntax. It works well enough for
GCC and Clang, but not for rustc.

This commit improves the parser a little more for better processing
comments, escape sequences, etc.

My main motivation is to drop comments. rustc may output comments
(e.g. env-dep). Currentyly, rustc build rules invoke sed to remove
comments, but it is more efficient to do it in fixdep.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/basic/fixdep.c | 173 ++++++++++++++++++++++++++++-------------
 1 file changed, 117 insertions(+), 56 deletions(-)

Comments

Miguel Ojeda Dec. 31, 2022, 3:25 p.m. UTC | #1
On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> My main motivation is to drop comments. rustc may output comments
> (e.g. env-dep). Currentyly, rustc build rules invoke sed to remove
> comments, but it is more efficient to do it in fixdep.

Hmm... I couldn't apply this one, it turns out `#include <stdarg.h>`
is gone. Adding it and adjusting the patch fixes it.

Cheers,
Miguel
Miguel Ojeda Dec. 31, 2022, 3:30 p.m. UTC | #2
On Sat, Dec 31, 2022 at 4:25 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Hmm... I couldn't apply this one, it turns out `#include <stdarg.h>`
> is gone. Adding it and adjusting the patch fixes it.

Ah, I think you created them on top of kbuild/fixes.

Cheers,
Miguel
Miguel Ojeda Jan. 3, 2023, 8:45 p.m. UTC | #3
On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This commit improves the parser a little more for better processing
> comments, escape sequences, etc.

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel
diff mbox series

Patch

diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 37782a632494..37a40520686f 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -94,6 +94,7 @@ 
 #include <unistd.h>
 #include <fcntl.h>
 #include <string.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <ctype.h>
@@ -251,75 +252,135 @@  static int is_ignored_file(const char *s, int len)
  * assignments are parsed not only by make, but also by the rather simple
  * parser in scripts/mod/sumversion.c.
  */
-static void parse_dep_file(char *m, const char *target)
+static void parse_dep_file(char *p, const char *target)
 {
-	char *p;
-	int is_last, is_target;
-	int saw_any_target = 0;
-	int is_first_dep = 0;
-	void *buf;
-
-	while (1) {
-		/* Skip any "white space" */
-		while (*m == ' ' || *m == '\\' || *m == '\n')
-			m++;
-
-		if (!*m)
-			break;
-
-		/* Find next "white space" */
-		p = m;
-		while (*p && *p != ' ' && *p != '\\' && *p != '\n')
-			p++;
-		is_last = (*p == '\0');
-		/* Is the token we found a target name? */
-		is_target = (*(p-1) == ':');
-		/* Don't write any target names into the dependency file */
-		if (is_target) {
-			/* The /next/ file is the first dependency */
-			is_first_dep = 1;
-		} else if (!is_ignored_file(m, p - m)) {
-			*p = '\0';
-
+	bool saw_any_target = false;
+	bool is_source = false;
+	bool searching_colon = true;
+	bool need_parse;
+	char *q, saved_c;
+
+	while (*p) {
+		/* handle some special characters first. */
+		switch (*p) {
+		case '#':
 			/*
-			 * Do not list the source file as dependency, so that
-			 * kbuild is not confused if a .c file is rewritten
-			 * into .S or vice versa. Storing it in source_* is
-			 * needed for modpost to compute srcversions.
+			 * skip comments.
+			 * rustc may emit comments to dep-info.
 			 */
-			if (is_first_dep) {
+			p++;
+			while (*p != '\0' && *p != '\n') {
 				/*
-				 * If processing the concatenation of multiple
-				 * dependency files, only process the first
-				 * target name, which will be the original
-				 * source name, and ignore any other target
-				 * names, which will be intermediate temporary
-				 * files.
+				 * escaped newlines continue the comment across
+				 * multiple lines.
 				 */
-				if (!saw_any_target) {
-					saw_any_target = 1;
-					printf("source_%s := %s\n\n",
-					       target, m);
-					printf("deps_%s := \\\n", target);
+				if (*p == '\\')
+					p++;
+				p++;
+			}
+			continue;
+		case ' ':
+		case '\t':
+			/* skip whitespaces */
+			p++;
+			continue;
+		case '\\':
+			/*
+			 * backslash/newline combinations continue the
+			 * statement. Skip it just like a whitespace.
+			 */
+			if (*(p + 1) == '\n') {
+				p += 2;
+				continue;
+			}
+			break;
+		case '\n':
+			/*
+			 * Makefiles use a line-based syntax, where the newline
+			 * is the end of a statement. After seeing a newline,
+			 * we expect the next token is a target.
+			 */
+			p++;
+			searching_colon = true;
+			continue;
+		case ':':
+			/*
+			 * assume the first dependency after a colon as the
+			 * source file.
+			 */
+			p++;
+			searching_colon = false;
+			is_source = true;
+			continue;
+		}
+
+		/* find the end of the token */
+		q = p;
+		while (*q != ' ' && *q != '\t' && *q != '\n' && *q != '#' && *q != ':') {
+			if (*q == '\\') {
+				if (*(q + 1) == '\n')
+					break;
+
+				/* escaped special characters */
+				if (*(q + 1) == '#' || *(q + 1) == ':') {
+					memmove(p + 1, p, q - p);
+					p++;
 				}
-				is_first_dep = 0;
-			} else {
-				printf("  %s \\\n", m);
+
+				q++;
 			}
 
-			buf = read_file(m);
-			parse_config_file(buf);
-			free(buf);
+			if (*q == '\0')
+				break;
+			q++;
 		}
 
-		if (is_last)
-			break;
+		/* Just discard the target */
+		if (searching_colon) {
+			p = q;
+			continue;
+		}
+
+		saved_c = *q;
+		*q = '\0';
+		need_parse = false;
 
 		/*
-		 * Start searching for next token immediately after the first
-		 * "whitespace" character that follows this token.
+		 * Do not list the source file as dependency, so that kbuild is
+		 * not confused if a .c file is rewritten into .S or vice versa.
+		 * Storing it in source_* is needed for modpost to compute
+		 * srcversions.
 		 */
-		m = p + 1;
+		if (is_source) {
+			/*
+			 * The DT build rule concatenates multiple dep files.
+			 * When processing them, only process the first source
+			 * name, which will be the original one, and ignore any
+			 * other source names, which will be intermediate
+			 * temporary files.
+			 */
+			if (!saw_any_target) {
+				saw_any_target = true;
+				printf("source_%s := %s\n\n", target, p);
+				printf("deps_%s := \\\n", target);
+				need_parse = true;
+			}
+		} else if (!is_ignored_file(p, q - p)) {
+			printf("  %s \\\n", p);
+			need_parse = true;
+		}
+
+		if (need_parse) {
+			void *buf;
+
+			buf = read_file(p);
+			parse_config_file(buf);
+			free(buf);
+		}
+
+		is_source = false;
+		*q = saved_c;
+		p = q;
 	}
 
 	if (!saw_any_target) {