Issue 74007 - stripping ./ from targets modifies $@
Summary: stripping ./ from targets modifies $@
Status: CLOSED FIXED
Alias: None
Product: Build Tools
Classification: Code
Component: dmake (show other issues)
Version: current
Hardware: All All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: hjs
QA Contact: issues@tools
URL:
Keywords:
Depends on:
Blocks: 77202
  Show dependency tree
 
Reported: 2007-01-30 09:13 UTC by hjs
Modified: 2013-08-07 15:34 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
New normpath function macro (1.73 KB, patch)
2007-05-22 23:07 UTC, quetschke
no flags Details | Diff
Add foo/bar => ./foo/bar transformation (4.47 KB, patch)
2007-05-28 23:48 UTC, quetschke
no flags Details | Diff
Add special OOo build mode to control the ./foo => foo normalization (4.52 KB, patch)
2007-06-03 16:59 UTC, quetschke
no flags Details | Diff
New patch using OOODMAKEMODE (4.52 KB, patch)
2007-06-20 23:27 UTC, quetschke
no flags Details | Diff
Man page patch (1015 bytes, patch)
2007-06-22 16:55 UTC, quetschke
no flags Details | Diff
Patch for dmake / OOODMAKEMODE (8.08 KB, patch)
2007-09-16 03:57 UTC, quetschke
no flags Details | Diff
Patch for dmake / OOODMAKEMODE (7.91 KB, patch)
2007-09-16 04:52 UTC, quetschke
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description hjs 2007-01-30 09:13:37 UTC
seems to be a result of #i69742# that holds some surprises:

--------------
mytarget=./something

$(mytarget):
    echo $(eq,$(mytarget),$@ equal different)
--------------

this is quite unintuitive and also a significant change from 4.6 to 4.7.

actually there is no known issue with this but it, for example, breaks
ZIPnTARGET in case of PRJ=. (see temporary filename). of cause this target can
be rewritten but i think this is rather a general problem...
Comment 1 quetschke 2007-01-31 01:08:35 UTC
Summarizing a little our conversation from IRC:

* To produce relocatable filenames it makes sense to normalize foo to ./foo and
  not the other way round. (This is actually easily doable if we want to do
  that.)

* We could introduce a new macro that holds the target name that was actually
  used in the target definition and not the normalized version. This would add
  some extra memory and would be moderately invasive.

* The previous solution could be applied to $@, but that would make .WINPATH
  (issue 73499) more or less unusable whenever it is most useful, i.e. for
  absolute paths.

Currently I would tend to only the first bullet if that is good enough, the
problem with it is that errors in string operations are hard to spot, for
example

--------------
mytarget=.//something

$(mytarget):
    echo $(eq,$(mytarget),$@ equal different)
--------------

would still not be equal because the legal, but unneeded second slash in
mytarget would normalized away. Or for
 mytarget=./bla/../something
the bla/.. would be normalized away.
Comment 2 quetschke 2007-05-22 20:34:36 UTC
Hmmm, I was just looking into issue 77202 to add a general mechanism foo => ./foo
but now I fail to see why ./foo is more relocatable than foo. I remember there
was an example but I forgot what it was.


For the example in this issue, wouldn't a function macro $(normalize ...) (it
normalizes a given macro as if it would be a target) solve most of our
problems?

Something like this would work:
--------------
mytarget=./something

$(mytarget):
    echo $(eq,$(normalize mytarget),$@ equal different)
--------------
Comment 3 quetschke 2007-05-22 23:07:26 UTC
Created attachment 45324 [details]
New normpath function macro
Comment 4 quetschke 2007-05-22 23:12:01 UTC
The previous patch implements a $(normpath ..) function macro that implements
a normalization of the given macro. It honors .WINPATH.

Maybe we should have two function macros, one that only normalizes to the internal
representation (not following .WINPATH) ( that should be $(normpath ..) ) and
another called one that follows .WINPATH ( should be called $(winpath ..) ).

Opinions?
Comment 5 quetschke 2007-05-23 23:24:22 UTC
I just briefly looked at the ZIPnTARGET problem, and I think the biggest problem
is this

"{$(subst,LANGDIR,{$(subst,$(ZIP$(TNR)TARGET)_, $(@:f:b))} $j )}"

part. So instead of using the foo => ./foo translation (for which I will prepare
a patch anyway) this could also be fixed by using normpath, i.e.:

"{$(subst,LANGDIR,{$(subst,$(normpath ZIP$(TNR)TARGET)_, $(@:f:b))} $j )}"

(Yes, I know, if we go down this road we will have patches all over solenv/inc )
Comment 6 quetschke 2007-05-28 23:48:43 UTC
Created attachment 45474 [details]
Add foo/bar => ./foo/bar transformation
Comment 7 quetschke 2007-05-29 00:07:43 UTC
The previous patch adds a general foo/bar => ./foo/bar transformation to work
around mentioned in this and issue 77202. I had to add the additional requirement
that at least one directory separator is present because otherwise some internal
targets and more importantly imported environment variables would also get ./
prepended. (A can of worms ...)

Contrary to what I said on IRC, I don't like this anymore, and the simple patch
from issue 77202 that only disables the stripping of leading ./ sounds like
the best way around this problem. (Maybe enabled by an command line switch for
an OOo special mode)

The correct version would still be the use of normpath before comparing and
substituting but that's a lot of work ...
Comment 8 quetschke 2007-06-03 16:59:09 UTC
Created attachment 45622 [details]
Add special OOo build mode to control the ./foo => foo  normalization
Comment 9 quetschke 2007-06-03 17:23:32 UTC
The previous patch introduces a new special macro that can be used to toggle
OOo build specific behavior. If OOOSPECIAL is set (means it begins with y)
the leading ./ of a path will no longer be removed.

The OOOSPECIAL value can be imported with .IMPORT

Example:
- - makefile.mk - -
SHELL*:=/bin/sh
SHELLFLAGS*:=-ce

.IMPORT : OOOSPECIAL

./all :
	@echo $@
- - - -

This will either echo ./all with OOOSPECIAL=yes or just all if OOOSPECIAL is not
set.

Is OOOSPECIAL a good macro name? Maybe we should call it OOODMAKEMODE?

(The patch also contains some extra clean-up and the normalization of leading
slashes, see issue 78061.)
Comment 10 quetschke 2007-06-20 23:27:14 UTC
Created attachment 46123 [details]
New patch using OOODMAKEMODE
Comment 11 quetschke 2007-06-20 23:28:03 UTC
I committed the previous patch, docu and testcase are still pending.
Comment 12 quetschke 2007-06-21 21:28:41 UTC
Testcase committed.
Comment 13 quetschke 2007-06-22 16:55:30 UTC
Created attachment 46190 [details]
Man page patch
Comment 14 quetschke 2007-06-22 18:39:25 UTC
Documantation change committed.

The $(normpath ..) function macro will be handled by issue 78776. Using the
OOODMAKEMODE macro should be sufficient to emulate the old dmake bahavior.
Comment 15 quetschke 2007-06-22 18:40:05 UTC
@ause: Please verify.
Comment 16 hjs 2007-06-26 18:24:54 UTC
.
Comment 17 hjs 2007-07-13 14:30:35 UTC
dmake 4.9 active in MWS
Comment 18 hjs 2007-09-04 17:52:59 UTC
OOODMAKEMODE gets evaluated too late when dealing with explicit targets from the
commandline.

dmake ./ttt

cuts "./" no matter if OOODMAKEMODE is set in environment or startup.mk. even

dmake ./ttt OOODMAKEMODE=YES

doesn't work while 

dmake OOODMAKEMODE=YES ./ttt

does...

no real surprise but quite annoing. any chance to get "./" cut off later (when
startup.mk is evaluated or so...)?


Comment 19 quetschke 2007-09-05 03:08:28 UTC
I admit OOODMAKEMODE is a cludge, it's just to work around a problem in the
makefiles that need the leading ./ . It "breaks" the normalizing of the internal
representation of targets. Unfortunately dmake just processes the commandline
arguments in the order of appearance, so that the macro definition of OOODMAKEMODE
has to happen before the target is created, and the target is created once it
appears on the commandline. It might be possible to save the list of targets
mentioned on the commandline and process them after startup.mk is read.

Is it worth the effort? I would vote for putting in the environment, that is
a one line patch for config_office and probably equally easy for the Hamburg
environment.
This would remind us of this problem, and maybe we can get of it at one point by
using $(SOMEVAR:n) or $(normpath ..).
Comment 20 hjs 2007-09-05 08:30:08 UTC
the problem is that even the environment settings (.IMPORT in startup.mk) are
evaluated too late. maybe makeing OOODMAKEMODE a special variable, like
DMAKEROOT(?) could help.

the actual problem i stumbled uppon (rules.mk) can be workarounded by inserting
OOODMAKEMODE=TRUE into each and every recursive call of dmake. but that wouldn't
help when using "./" targets on the commandline (beside being not very intuitive ;))
Comment 21 quetschke 2007-09-05 16:16:58 UTC
Ah, I didn't realize that setting the environment variable doesn't help
because the .IMPORT : .EVERYTHING does happen in startup.mk, or never if
it is not set.

Yes, I can treat OOODMAKEMODE like DMAKEROOT. Would this solution be acceptable
if it works? (I'll test later)

If that fails I will try to move the "definition" of the command line targets
behind the evaluation of startup.
Comment 22 quetschke 2007-09-16 03:57:37 UTC
Created attachment 48252 [details]
Patch for dmake / OOODMAKEMODE
Comment 23 quetschke 2007-09-16 04:27:43 UTC
Patch and testcase committed.

Now OOODMAKEMODE is *always* imported from the environment and the targets from
the command line are defined after OOODMAKEMODE is read from the environment and
after the macros from the command line are set.
Comment 24 quetschke 2007-09-16 04:52:55 UTC
Created attachment 48253 [details]
Patch for dmake / OOODMAKEMODE
Comment 25 quetschke 2007-09-16 05:18:14 UTC
The previous patch reverts the part of the previous patch that lets OOODMAKEMODE
always be imported from the environment. Instead move the definition of targets
from the command line after the evaluation of the startup makefile.

Committed and testcase changed.

Please verify
Comment 26 hjs 2007-10-11 18:00:08 UTC
works as described
Comment 27 hjs 2007-12-18 12:45:06 UTC
.