bugmake - Bugs: bug #62706, Restrict second expansion to...

 
 

bug #62706: Restrict second expansion to targets which are being built.

Submitter:  Dmitry Goncharov <dgoncharov>
Submitted:  Mon 04 Jul 2022 11:19:17 PM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  Fixed Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  4.4 Operating System:  Any
Fixed Release:  4.4 Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 31 Jul 2022 02:29:05 AM UTC, comment #6: 

Applied these changes; Thanks!

Paul D. Smith <psmith>
Group administrator
Sat 23 Jul 2022 01:59:41 PM UTC, comment #5: 

sv62706_fix2.diff takes care of secondary expanding a prerequisite marked as intermediate by .SECONDARY or .INTERMEDIATE when an explicit or static pattern rule builds the prerequisite.
sv62706_test2.diff adds related tests.

Dmitry Goncharov <dgoncharov>
Sat 09 Jul 2022 07:59:26 PM UTC, comment #4: 

Since secondary expansion is not defined in POSIX it doesn't really matter what POSIX says about this, IMO: we could easily define it either way.

However, I think this change is correct and the way I would expect things to work, so it's a good one.

Paul D. Smith <psmith>
Group administrator
Sat 09 Jul 2022 07:30:55 PM UTC, comment #3: 

Another thought on point 2 above.
Posix contains the following requirement.

"The make utility shall treat all prerequisites as targets themselves and recursively ensure that they are up-to-date, processing them in the order in which they appear in the rule."

Should word "processing" here be interpreted as any activity, including secondary expansion (which posix does not define), needed to bring a prerequisite up to date?

Secondary expansion of implicit rules in gnu make already satisfied these requirements. This patch makes explicit and static pattern rules to satisfy as well.

Dmitry Goncharov <dgoncharov>
Tue 05 Jul 2022 06:33:46 PM UTC, comment #2: 

This is a more detailed description of point 2 above (2. Causes all prerequisites to be second expanded in the same order they are being built.)


$ cat makefile
.SECONDEXPANSION:
all: hello.tsk bye.tsk
hello.tsk: $$(info 2nd expansion of prereqs of $$@); $(info $@)
bye.tsk: $$(info 2nd expansion of prereqs of $$@); $(info $@)
$ # this is patched make
$ ~/src/make/l64/make
2nd expansion of prereqs of hello.tsk
hello.tsk
2nd expansion of prereqs of bye.tsk
bye.tsk
make: Nothing to be done for 'all'.
$ # this is make-4.3
$ make
2nd expansion of prereqs of bye.tsk
2nd expansion of prereqs of hello.tsk
hello.tsk
bye.tsk
make: Nothing to be done for `all'.
$


We can see that make-4.3 builds 'hello.tsk' before building
'bye.tsk'. The order of secondary expanding prerequisites is the
opposite, first the prerequisites of 'bye.tsk' are expanded, then the
prerequisites of 'hello.tsk'.

The proposed behavior is to secondary expand the prerequisites of
'hello.tsk", build 'hello.tsk', secondary expand the prerequisites of
'bye.tsk', build 'bye.tsk'.

If however the makefile is changed like

$ cat makefile
.SECONDEXPANSION:
all: $$(info 2nd expansion for all) hello.tsk bye.tsk; $(info $@)
hello.tsk: $$(info 1st 2nd expansion of prereqs of $$@) $$(info 2nd 2nd expansion of prereqs of $$@); $(info $@)
bye.tsk: $$(info 1st 2nd expansion of prereqs of $$@) $$(info 2nd 2nd expansion of prereqs of $$@); $(info $@)
$ ~/src/make/l64/make
2nd expansion for all
1st 2nd expansion of prereqs of hello.tsk
2nd 2nd expansion of prereqs of hello.tsk
hello.tsk
1st 2nd expansion of prereqs of bye.tsk
2nd 2nd expansion of prereqs of bye.tsk
bye.tsk
all
make: 'all' is up to date.
$ make
2nd expansion for all
1st 2nd expansion of prereqs of bye.tsk
2nd 2nd expansion of prereqs of bye.tsk
1st 2nd expansion of prereqs of hello.tsk
2nd 2nd expansion of prereqs of hello.tsk
hello.tsk
bye.tsk
all
make: `all' is up to date.
$

Then $$(info 2nd expansion for all) is secondary expanded
before the prerequisites of hello.tsk and bye.tsk are secondary
expanded. The patch keeps this aspect of behavior is intact.

Dmitry Goncharov <dgoncharov>
Mon 04 Jul 2022 11:42:51 PM UTC, comment #1: 

Second expand only the prerequisites of the targets being built.
Avoid second expanding the prerequisites of targets which are not being built.

There are multiple benefits.

1. Avoids redundant work of second expanding prerequisites of the targets which are not being built.

Example 1

.SECONDEXPANSION:
hello.tsk: $$(info 2nd exp for hello) $$(file <hello.d); $(info $@ from $^)
bye.tsk: $$(info 2nd exp for bye) $$(file <bye.d); $(info $@ from $^)
clean:; -rm -f hello.tsk bye.tsk


Assuming there are files 'hello.d' and 'bye.d' which contain the prerequisites of 'hello.tsk' and 'bye.tsk' respectively


$ make hello.tsk

second expands $$(file <hello.d), but does not second expand $$(file bye.d).



$ make clean

does not second expand either $$(file <hello.d) or $$(file <bye.d).


2. Causes all prerequisites to be second expanded in the same order they are being built. This fixes byte order dependent order of second expanding prerequisites. See sv 62175.

3. Eliminates redundant files from the make's database, which reduces the size of the hash table and memory consumption.

4. Prevents side effects from second expansion of unrelated prerequisites.  Even though we'd prefer makefiles which do not depend on side effects, the current behavior allows them.

Example 2

.SECONDEXPANSION:
hello.tsk:; cp hello.1 $@
unrelated: $$(shell touch hello.1);


The current behavior is to second expand $$(shell touch hello.1) during parse phase and thus create 'hello.1'. This allows 'cp hello.1 $@' to succeed.  The new behavior is such that when 'hello.tsk' is built, $$(shell touch hello.1) is not second expanded, and thus $$(shell touch hello.1) fails.

Another size effect is that, because the prerequisites of unrelated targets are not second expanded, those prerequisites may end up being treated as intermediates, where before this commit they would be treated as explicitly defined.

Example 3

.SECONDEXPANSION:
dep:=hello.x
all: hello.z
%.z: %.x; @echo $@
%.x: ;
unrelated: $$(dep)


In example 3 the old behavior is to second expand $$(dep) at parse time and thus enter file 'hello.x' to make's database prevent 'hello.x' from being intermediate. The new behavior is $$(dep) is not expanded and 'hello.x' is not entered and thus 'hello.x' is intermediate.

Both of this examples are artificially created to demonstrate side effects and are not very realistic.  Normally, an automatic variable would be second expanded, otherwise, there is really no point to second expand prerequisites at all.

i checked automake and cmake and found no dependency on such side effects. As far as i can see, most projects do not use second expansion.

However, if make has to retain side effects, it is possible to introduce another special target with the new behavior.



5. Implicit rules have behaved in this manner since day 1. This patch causes explicit and static pattern rules behave like implicit rules and do the required work only.

Dmitry Goncharov <dgoncharov>
Mon 04 Jul 2022 11:19:17 PM UTC, original submission:  

.

Dmitry Goncharov <dgoncharov>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53465:  sv62706_fix2.diff added by dgoncharov (6KiB - text/x-patch)
file #53466:  sv62706_test2.diff added by dgoncharov (16KiB - text/x-patch)
file #53396:  sv62706_doc.diff added by dgoncharov (4KiB - text/x-patch)
file #53397:  sv62706_test.diff added by dgoncharov (15KiB - text/x-patch)
file #53398:  sv62706_fix.diff added by dgoncharov (5KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dgoncharov (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-07-31 psmith Fixed ReleaseNone 4.4
    2022-07-31 psmith StatusNone Fixed
        Open/ClosedOpen Closed
    2022-07-23 dgoncharov Attached File- Added sv62706_fix2.diff, #53465
        Attached File- Added sv62706_test2.diff, #53466
    2022-07-04 dgoncharov Attached File- Added sv62706_doc.diff, #53396
        Attached File- Added sv62706_test.diff, #53397
        Attached File- Added sv62706_fix.diff, #53398

    Back to the top

    Powered by Savane 3.13-da57.
    Corresponding source code