On Sat, May 15, 2021 at 10:23:31PM +0200, Ingo Schwarze wrote: > Hi Klemens, > > Klemens Nanni wrote on Mon, Apr 05, 2021 at 09:33:13PM +0200: > > On Mon, Apr 05, 2021 at 06:47:58PM +0200, Ingo Schwarze wrote: > >> Klemens Nanni wrote on Sun, Apr 04, 2021 at 03:54:43PM +0200: > >>> On Sun, Apr 04, 2021 at 03:42:03PM +0200, Tim van der Molen wrote: > > >>>> Doesn't mandoc -Tlint -Wstyle do what you want? > > >>> I don't think so. Oddly enough, `-Wstyle' does not do what I would > >>> expect: it omits STYLE messages instead of printing them. > > >> It does not. It only omits BASE messages, which is the lowest message > >> level and the only one below STYLE. See `man -O tag=base mandoc` > >> what the purpose of BASE is. > [...] > > >>> But it would also suppress legitimate messages, e.g. `.Xr' macros with > >>> misspelled manuals in the base MANPATH: > > >> Yes, so far, checking of .Xr targets is only done on the BASE level, > >> not on the STYLE level because i regarded the requirement that .Xr targets > >> exist as a requirement of the OpenBSD base system. For portable software > >> outside the OpenBSD base system, in the past, i didn't think checking .Xr > >> targets would be helpful. It appears you now found a use case for that. > > > A use case, but not really a requirement, at least not for people > > developing portable software on OpenBSD -- that's where I'm coming from: > > messages telling me installed manuals cannot be found are annoying. > > > > But I guess that turns into a valid use case as soon as mandoc is used > > on and for another system with different manual paths. > > >> Your patch is not OK as it stands because mandoc would no longer help > >> developers working on the base system to find .Xr links pointing > >> outside the base system, and we have a policy that we don't want those. > > > Agreed. > > >> I guess the right thing to do is to leave "-W all" unchanged (including > >> looking in the base system only) but change "-W style" to also do the .Xr > >> target check, but along the user's manpath. > > > That makes sense and the diff below enables check_xr() to pick the right > > manual paths. > > > > HOWEVER it currently won't print anything because when `-Wstyle' is used > > mandoc_msg(MANDOCERR_XR_BAD, ...) in check_xr() won't print anything as > > MANDOC_XR_BAD is lower than MANDOCERR_STYLE. > > > > I played swapping those two levels in the `enum mandocerr' but that > > won't quite work (e.g. mandoc_msg() printing the wrong message)... > > > > Ingo, perhaps you have an idea on how to fix that? > > Does the following patch work for you?
Yes, because this makes `-Tlint -Wstyle' do the right thing, i.e. only warn about missing manuals if they are in fact missing from MANPATH. This way I can lint non-base manuals, get all the style warnings, but now actually without the .Xr false positives. > Here is what it does: > > * parse the config file not only in man(1) and apropos(1) mode, > but also in mandoc(1) -T style mode because the full manpath > is now needed for .Xr checking even when the manual page file > to be processed does not need to be searched for but is provided > on the command line > > * pass through configuration data to the various checking functions > as far as they now need it; > > * move MANDOCERR_XR_BAD from BASE to STYLE both in the enum (mandoc.h) > and in the message catalog (mandoc_msg.c) > > * properly document the change Sounds sane, manual change is clear and the diff works for me (I've tested against base and non-base manuals, both referencing existent as well as nonexistent manuals). Thanks, OK kn > Index: main.c > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/main.c,v > retrieving revision 1.256 > diff -u -p -r1.256 main.c > --- main.c 19 Feb 2021 19:49:49 -0000 1.256 > +++ main.c 15 May 2021 20:04:51 -0000 > @@ -1,6 +1,6 @@ > /* $OpenBSD: main.c,v 1.256 2021/02/19 19:49:49 kn Exp $ */ > /* > - * Copyright (c) 2010-2012, 2014-2020 Ingo Schwarze <[email protected]> > + * Copyright (c) 2010-2012, 2014-2021 Ingo Schwarze <[email protected]> > * Copyright (c) 2008-2012 Kristaps Dzonsons <[email protected]> > * Copyright (c) 2010 Joerg Sonnenberger <[email protected]> > * > @@ -92,7 +92,7 @@ struct outstate { > > int mandocdb(int, char *[]); > > -static void check_xr(void); > +static void check_xr(struct manpaths *); > static int fs_lookup(const struct manpaths *, > size_t ipath, const char *, > const char *, const char *, > @@ -103,7 +103,7 @@ static int fs_search(const struct man > static void glob_esc(char **, const char *, const char *); > static void outdata_alloc(struct outstate *, struct > manoutput *); > static void parse(struct mparse *, int, const char *, > - struct outstate *, struct manoutput *); > + struct outstate *, struct manconf *); > static void passthrough(int, int); > static void process_onefile(struct mparse *, struct > manpage *, > int, struct outstate *, struct manconf *); > @@ -430,7 +430,8 @@ main(int argc, char *argv[]) > > /* Read the configuration file. */ > > - if (search.argmode != ARG_FILE) > + if (search.argmode != ARG_FILE || > + mandoc_msg_getmin() == MANDOCERR_STYLE) > manconf_parse(&conf, conf_file, defpaths, auxpaths); > > /* man(1): Resolve each name individually. */ > @@ -841,7 +842,7 @@ process_onefile(struct mparse *mp, struc > } > > if (resp->form == FORM_SRC) > - parse(mp, fd, resp->file, outst, &conf->output); > + parse(mp, fd, resp->file, outst, conf); > else { > passthrough(fd, conf->output.synopsisonly); > outst->had_output = 1; > @@ -862,8 +863,9 @@ process_onefile(struct mparse *mp, struc > > static void > parse(struct mparse *mp, int fd, const char *file, > - struct outstate *outst, struct manoutput *outconf) > + struct outstate *outst, struct manconf *conf) > { > + static struct manpaths basepaths; > static int previous; > struct roff_meta *meta; > > @@ -889,7 +891,7 @@ parse(struct mparse *mp, int fd, const c > return; > > if (outst->outdata == NULL) > - outdata_alloc(outst, outconf); > + outdata_alloc(outst, &conf->output); > else if (outst->outtype == OUTT_HTML) > html_reset(outst->outdata); > > @@ -946,24 +948,25 @@ parse(struct mparse *mp, int fd, const c > break; > } > } > - if (outconf->tag != NULL && outconf->tag_found == 0 && > - tag_exists(outconf->tag)) > - outconf->tag_found = 1; > - if (mandoc_msg_getmin() < MANDOCERR_STYLE) > - check_xr(); > + if (conf->output.tag != NULL && conf->output.tag_found == 0 && > + tag_exists(conf->output.tag)) > + conf->output.tag_found = 1; > + > + if (mandoc_msg_getmin() < MANDOCERR_STYLE) { > + if (basepaths.sz == 0) > + manpath_base(&basepaths); > + check_xr(&basepaths); > + } else if (mandoc_msg_getmin() < MANDOCERR_WARNING) > + check_xr(&conf->manpath); > } > > static void > -check_xr(void) > +check_xr(struct manpaths *paths) > { > - static struct manpaths paths; > struct mansearch search; > struct mandoc_xr *xr; > size_t sz; > > - if (paths.sz == 0) > - manpath_base(&paths); > - > for (xr = mandoc_xr_get(); xr != NULL; xr = xr->next) { > if (xr->line == -1) > continue; > @@ -972,9 +975,9 @@ check_xr(void) > search.outkey = NULL; > search.argmode = ARG_NAME; > search.firstmatch = 1; > - if (mansearch(&search, &paths, 1, &xr->name, NULL, &sz)) > + if (mansearch(&search, paths, 1, &xr->name, NULL, &sz)) > continue; > - if (fs_search(&search, &paths, xr->name, NULL, &sz) != -1) > + if (fs_search(&search, paths, xr->name, NULL, &sz) != -1) > continue; > if (xr->count == 1) > mandoc_msg(MANDOCERR_XR_BAD, xr->line, > Index: mandoc.1 > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/mandoc.1,v > retrieving revision 1.174 > diff -u -p -r1.174 mandoc.1 > --- mandoc.1 4 Apr 2021 06:18:58 -0000 1.174 > +++ mandoc.1 15 May 2021 20:04:51 -0000 > @@ -1,6 +1,6 @@ > .\" $OpenBSD: mandoc.1,v 1.174 2021/04/04 06:18:58 jmc Exp $ > .\" > -.\" Copyright (c) 2012, 2014-2020 Ingo Schwarze <[email protected]> > +.\" Copyright (c) 2012, 2014-2021 Ingo Schwarze <[email protected]> > .\" Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <[email protected]> > .\" > .\" Permission to use, copy, modify, and distribute this software for any > @@ -922,14 +922,6 @@ generated by CVS > or > .Ic NetBSD > keyword substitution as conventionally used in these operating systems. > -.It Sy "referenced manual not found" > -.Pq mdoc > -An > -.Ic \&Xr > -macro references a manual page that is not found in the base system. > -The path to look for base system manuals is configurable at compile > -time and defaults to > -.Pa /usr/share/man : Ns Pa /usr/X11R6/man . > .El > .Ss Style suggestions > .Bl -ohang > @@ -1016,6 +1008,35 @@ list contains two consecutive > entries describing the same > .Ic \&Er > number. > +.It Sy "referenced manual not found" > +.Pq mdoc > +An > +.Ic \&Xr > +macro references a manual page that was not found. > +When running with > +.Fl W Cm base , > +the search is restricted to the base system, by default to > +.Pa /usr/share/man : Ns Pa /usr/X11R6/man . > +This path can be configured at compile time using the > +.Dv MANPATH_BASE > +preprocessor macro. > +When running with > +.Fl W Cm style , > +the search is done along the full search path as described in the > +.Xr man 1 > +manual page, respecting the > +.Fl m > +and > +.Fl M > +command line options, the > +.Ev MANPATH > +environment variable, the > +.Xr man.conf 5 > +file and falling back to the default of > +.Pa /usr/share/man : Ns Pa /usr/X11R6/man : Ns Pa /usr/local/man , > +also configurable at compile time using the > +.Dv MANPATH_DEFAULT > +preprocessor macro. > .It Sy "trailing delimiter" > .Pq mdoc > The last argument of an > Index: mandoc.h > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v > retrieving revision 1.211 > diff -u -p -r1.211 mandoc.h > --- mandoc.h 1 Sep 2020 18:24:09 -0000 1.211 > +++ mandoc.h 15 May 2021 20:04:51 -0000 > @@ -54,7 +54,6 @@ enum mandocerr { > MANDOCERR_ARCH_BAD, /* unknown architecture: Dt ... arch */ > MANDOCERR_OS_ARG, /* operating system explicitly specified: Os ... */ > MANDOCERR_RCS_MISSING, /* RCS id missing */ > - MANDOCERR_XR_BAD, /* referenced manual not found: Xr name sec */ > > MANDOCERR_STYLE, /* ===== start of style suggestions ===== */ > > @@ -68,6 +67,7 @@ enum mandocerr { > MANDOCERR_BX, /* consider using OS macro: macro */ > MANDOCERR_ER_ORDER, /* errnos out of order: Er ... */ > MANDOCERR_ER_REP, /* duplicate errno: Er ... */ > + MANDOCERR_XR_BAD, /* referenced manual not found: Xr name sec */ > MANDOCERR_DELIM, /* trailing delimiter: macro ... */ > MANDOCERR_DELIM_NB, /* no blank before trailing delimiter: macro ... */ > MANDOCERR_FI_SKIP, /* fill mode already enabled, skipping: fi */ > Index: mandoc_msg.c > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/mandoc_msg.c,v > retrieving revision 1.10 > diff -u -p -r1.10 mandoc_msg.c > --- mandoc_msg.c 1 Sep 2020 18:24:10 -0000 1.10 > +++ mandoc_msg.c 15 May 2021 20:04:51 -0000 > @@ -53,7 +53,6 @@ static const char *const type_message[MA > "unknown architecture", > "operating system explicitly specified", > "RCS id missing", > - "referenced manual not found", > > "generic style suggestion", > > @@ -67,6 +66,7 @@ static const char *const type_message[MA > "consider using OS macro", > "errnos out of order", > "duplicate errno", > + "referenced manual not found", > "trailing delimiter", > "no blank before trailing delimiter", > "fill mode already enabled, skipping", >
