Re: [PATCH] find RLIM_NLIMITS correctly on Cygwin

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Jun T wrote on Tue, 25 Feb 2020 18:38 +0900:
> Sorry for a *very* slow response; I've been quite busy these days.
> Finally I found some time to work on this.

No worries; we've all been there.  I'm swamped/backlogged too.

> But removing rlimits.awk has one drawback:
> Before the patch, unknown resource can be detected in the build
> time (by the special make rule defined in rlimits.mdd).
> With the patch, we can notice unknown resouce(s) only when running
> the limit (or ulimit) builtin. We may hope that a user who sees
> UNKNOWN in the output of limit/ulimit will report it to us, or
> add a simple test like
> 
> limit | grep UNKNOWN || print OK
> 
> somewhere in the test (but in which .ztst file?).

I prefer a new test, but to arrange things in such a way that a failure
of the new test won't fail the build or prevent other tests from
running.

So, how about: —

1. Placing the test in a new file, V99nonfatal.ztst; and

2. Using «F:» lines to explain that when the test fails, that shouldn't
be treated as a build breaker and there is no problem to go on and do
«make install».

?

We might even add a new flag, e.g., «ZTST_failure_is_fatal=false», which
would cause «make test» to report the failure on stderr but keep the
exit code as zero: this way, if someone builds zsh 5.8 five years from
now on a 2025-vintage OS, their «make && make check && make install»
wouldn't fail on account of OS API changes between now and then.

A couple of minor issues spotted in read-through:

> --- a/Src/Builtins/rlimits.c
> +++ b/Src/Builtins/rlimits.c
> @@ -53,11 +40,214 @@ enum {
> +typedef struct {

Suggest to name the struct type — «typedef struct resinfo_t { … }
resinfo_t» — as that tends to result in prettier output in the debugger.

> +/* table of known resources */
> +static resinfo_t known_resources[] = {

Suggest to make the variable const.

I haven't had time yet to do a full review or to test this, I'm afraid ☹

Cheers,

Daniel