[m-rev.] systematic problem with tests/valid* for C#

[Zoltan Somogyi]

On 2023-10-04 16:11 +11:00 AEDT, "Peter Wang" <novalazy at gmail.com> wrote:
>> Get "make cs"/"mmc --make x.cs" build a C# file ...
>> 
>> ... instead of building a bunch of .c files.
>> 
>> Our tradition of adding an "s" at the end of a suffix to mean "all of the
>> files with the original suffix" had a problem when we added C# as a target
>> language. Until then, just as "os" stood for ".o files" when it occurred
>> as either a mmake target, mmc --make target, or mmake variable name component.
>> "cs" likewise stood for ".c files", but was now also needed to mean ".cs file".
>> We coped by keeping "cs" meaning ".c files", and adding "csharp" as a target
>> name synonym to mean ".cs file".
>> 
>> This diff keeps that synonym, but it changes
>> 
>> - the name needed to refer to ".c files" from "cs" to "all_cs"
>> - the name needed to refer to ".o files" from "os" to "all_os"
>> - the name needed to refer to ".pic_o files" from "pic_os" to "all_pic_os"
>> - the name needed to refer to ".cs files" from "css" to "all_css"
>> - the name needed to refer to ".java files" from "javas" to "all_javas"
>> - the name needed to refer to ".opt files" from "opts" to "all_opts"
>> - the name needed to refer to ".trans_opt files"
>>         from "trans_opts" to "all_trans_opts"
> 
> (It's too bad that .css is a common file extension as well.
> At least the Mercury compiler is unlikely ever to generate CSS files.)

Well, we don't use .css by itself anymore: it is .all_css now.
 
> Apart from the Mercury system itself, I doubt that users are depending
> on the variables in .dep files.

Agreed, but ...

> AFAICT they are not documented either.

... this does not *guarantee* the absence of users who depend
on visible-though-not-documented parts of the system.
 
>> tools/bootcheck:
> 
> You didn't write anything for bootcheck.

Answered in my reply to Julien.

>> +  We have now changed the meaning of target `name.cs` to mean
>> +  `build the .cs file of the named module`. If you want to build
>> +  all the .c files of a program, you can specify `program.all_cs`
>> +  as the target.
>> +
> 
> Replace the backticks around `build the .cs file of the named module`,
> otherwise it will be rendered in a monospace font.

Done.

> I think the rationale is too long for an inconsequential change.
> The only mention of a "all [type] files" target in the user guide
> is to a 'MAIN-MODULE.all_ints' target. It should suffice to say:

I agree, but I wanted this info available, both for you as reviewers,
and in the repository, in case we need this info in the future.

I will therefore commit the change to NEWS.md as is (with the quoting
fix above), and will then replace it with your shorter replacements
in a followup commit.

> The NEWS file for each release is rather long already. My experience
> with reading the changelogs of other projects is that the longer it is
> and filled with irrelevant (to me) minutia, the more likely I will not
> bother reading it. We should avoid expending too many words on each
> change, but especially on uninteresting changes.

Agreed.

>> @@ -379,9 +379,20 @@ lib%.install_library: lib%.$A lib%.$(EXT_FOR_SHARED_LIB) \
>>  lib%.install_grades:
>>  	rm -rf tmp_dir && \
>>  	mkdir tmp_dir && \
>> -	grade_files="$(foreach ext,$(GRADE_SUBDIR_EXTS),$($*.$(ext)s))" && \
>> +	grade_vars_o="$(foreach ext,$(GRADE_SUBDIR_EXTS),$*.$(ext)s)" && \
>> +	grade_vars="$(foreach mve,$(GRADE_SUBDIR_MVEXTS),$*.$(mve))" && \
>> +	grade_files_o="$(foreach ext,$(GRADE_SUBDIR_EXTS),$($*.$(ext)s))" && \
>> +	grade_files="$(foreach mve,$(GRADE_SUBDIR_MVEXTS),$($*.$(mve)))" && \
>> +	echo "grade_vars_o" >> /tmp/XYZZY \
>> +	echo "$$grade_vars_o" >> /tmp/XYZZY \
>> +	echo "grade_vars" >> /tmp/XYZZY \
>> +	echo "$$grade_vars" >> /tmp/XYZZY \
>> +	echo "grade_files_o" >> /tmp/XYZZY \
>> +	echo "$$grade_files_o" >> /tmp/XYZZY \
>> +	echo "grade_files" >> /tmp/XYZZY \
>> +	echo "$$grade_files" >> /tmp/XYZZY \
> 
> I assume you mean to delete these lines.

I have deleted them.

Thanks for the review.

Zoltan.