On Mon, Apr 21, 2025 at 10:25:47AM -0500, Eric Blake via M4-patches wrote:
> From: David Warme <[email protected]>
>
> Add ability to generate make dependency rules.
> * doc/m4.texinfo: Document new options.
> * src/builtin.c: Record dependencies for include() / sinclude().
> * src/m4.c: Add new options. Record dependencies for files
> specified on the command line. Generate dependencies at end.
> * src/m4.h: Add defines for tracking how files are references, and
> new functions for recording / generation of dependencies.
> * src/path.c: Added recording / generation of dependencies.
> * NEWS: Added a blurb for this new feature.
> * THANKS Added us to the list.
> Co-authored-by: Lorenzo Di Gregorio <[email protected]>
> ---
>
> This is NOT yet ready for inclusion - I insist that we NEED to have
> something that tests the functionality during 'make check' to ensure
> it doesn't regress. And there are probably other nitpicks that I will
> clean up (such as ChangeLog style, error message tweaks, figuring out
> the preferred way to do the flexible allocation of the file name in
> storage alongside 'struct dependency', and so forth). However, I'm
> reposting David's original patch as tentatively rebased to branch-1.6,
> to make it easier to continue to get this ready for inclusion before I
> cut the next beta release in preparation for an eventual 1.6.
Among other things, 'make syntax-check' fails:
> +++ b/doc/m4.texi
> @@ -145,6 +145,7 @@ Top
>
> * Operation modes:: Command line options for operation modes
> * Preprocessor features:: Command line options for preprocessor
> features
> +* Make dependency generation:: Generating make depencency rules
Typo: s/depencency/dependency/
> +++ b/src/builtin.c
> @@ -1472,13 +1472,14 @@ m4_changecom (struct obstack *obs MAYBE_UNUSED, int
> argc,
> argument, if it exists. Complain about inaccessible files iff
> SILENT is false. */
> static void
> -include (int argc, macro_arguments *argv, bool silent)
> +include (int argc, macro_arguments *argv, bool silent, int ref_from)
Should ref_from be an enum instead of int argument, since there are
only a finite number of callers?
> +++ b/src/m4.c
> @@ -365,6 +421,21 @@ static const struct option long_options[] = {
> {"warn-macro-sequence", optional_argument, NULL,
> WARN_MACRO_SEQUENCE_OPTION},
>
> + {"makedep", required_argument, NULL, MAKEDEP_OPTION},
> + {"makedep-target", required_argument, NULL, MAKEDEP_TARGET_OPTION},
> + {"makedep-gen-missing-argfiles", no_argument, NULL,
> + MAKEDEP_GEN_MISSING_ARGFILES_OPTION},
> + {"makedep-gen-missing-include", no_argument, NULL,
> + MAKEDEP_GEN_MISSING_INCLUDE_OPTION},
> + {"makedep-gen-missing-sinclude", no_argument, NULL,
> + MAKEDEP_GEN_MISSING_SINCLUDE_OPTION},
> + {"makedep-gen-missing-all", no_argument, NULL,
> + MAKEDEP_GEN_MISSING_ALL_OPTION},
> + {"makedep-phony-argfiles", no_argument, NULL,
> MAKEDEP_PHONY_ARGFILES_OPTION},
> + {"makedep-phony-include", no_argument, NULL, MAKEDEP_PHONY_INCLUDE_OPTION},
> + {"makedep-phony-sinclude", no_argument, NULL,
> MAKEDEP_PHONY_SINCLUDE_OPTION},
Long lines need wrapping. 'make syntax-check' runs GNU indent which
wants to fit things in 80 columns; I'm inclined to accept its
suggestions, except that:
> + else if (!makedep_path != !makedep_target)
> + m4_error (EXIT_FAILURE, 0, NULL,
> + _("--makedep and --makedep-target cannot be used
> independently"));
here it suggested:
m4_error (EXIT_FAILURE, 0, NULL,
- _("--makedep and --makedep-target cannot be used
independently"));
+ _
+ ("--makedep and --makedep-target cannot be used independently"));
which is wrong. The workaround there may be a shorter message:
_("--makedep must be used with --makedep-target")
> +++ b/src/path.c
> @@ -36,6 +36,18 @@ typedef struct includes includes;
> static includes *dir_list; /* the list of path directories */
> static includes *dir_list_end; /* the end of same */
> static int dir_max_length; /* length of longest directory name */
> +
> +struct dependency
> +{
> + struct dependency *next; /* next in list of dependencies */
> + int ref_from; /* bit mask: places file referenced from */
> + char path [1]; /* pathname of this dependency */
I still need to research the best way to do this; but if we keep it
with [1], GNU indent does not like the space before [.
> +};
> +
> +typedef struct dependency dependency;
> +
> +static dependency *dependency_list; /* the list of dependencies */
> +static dependency *dependency_list_end; /* the end of same */
>
>
> void
> @@ -194,6 +206,74 @@ m4_path_search (const char *file, bool binary, char
> **result)
> return fp;
> }
>
> +void
> +record_dependency (const char * path, int ref_from)
const char *path
> +{
> + dependency *dp;
> + size_t len, nbytes;
> + for (dp = dependency_list; dp != NULL; dp = dp->next)
> + if (strcmp (path, dp->path) == 0)
> + {
> + /* Remember all the places this file has been referenced from. */
> + dp->ref_from |= ref_from;
> + return;
> + }
> +
> + len = strlen (path);
> + nbytes = offsetof(dependency, path[0]) + len + 1;
offsetof (dependency
> + dp = xmalloc (nbytes);
> + dp->next = NULL;
> + dp->ref_from = ref_from;
> + strcpy (dp->path, path);
> +
> + if (dependency_list_end == NULL)
> + dependency_list = dp;
> + else
> + dependency_list_end->next = dp;
> + dependency_list_end = dp;
> +}
> +
> +void
> +generate_make_dependencies (const char *path, const char *target, int phony)
> +{
> + FILE *fp;
> + int col, len, maxcol;
Anything populated with strlen() should probably be size_t, not int.
> + dependency *dp;
> +
> + fp = fopen (path, "w");
> + if (!fp)
> + {
> + m4_error (0, errno, NULL, "unable to open %s",
> + quotearg_style_mem (locale_quoting_style, path, strlen
> (path)));
Long line.
> + return;
> + }
> +
> + /* Generate the main dependency rule. */
> + maxcol = 78;
> + fprintf (fp, "%s:", target);
> + col = strlen (target) + 1;
> + for (dp = dependency_list; dp != NULL; dp = dp->next)
> + {
> + len = 1 + strlen (dp->path);
Should we store len in 'struct dependency' so we aren't recomputing
strlen() each time a new lookup is done?
> + if (col + len + 2 > maxcol) /* +2 is for trailing space/backslash */
> + {
> + fprintf (fp," \\\n ");
fp, " \\\n "
Using fprintf to print something without % is overkill; fputs is
better. Why is the next line starting with a space here?
> + col = 1;
> + }
> + fprintf (fp, " %s", dp->path);
> + col += len;
> + }
> + fputs ("\n", fp);
> +
> + /* Generate phony targets for user-specified subset of dependencies. */
> + if (phony != 0)
> + for (dp = dependency_list; dp != NULL; dp = dp->next)
> + if ((dp->ref_from & phony) != 0)
> + fprintf (fp, "\n%s:\n", dp->path);
> +
> + fclose (fp);
I really have not yet inpsected the output files to make sure they
look sane; should they include any header (such as "generated by ...")
to make it obvious they were computed by a run of m4? This is why I
insist that there be testsuite coverage.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
M4-patches mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/m4-patches
