View Issue Details

IDProjectCategoryView StatusLast Update
0036688FPCCompilerpublic2020-02-20 10:16
ReporterAlfredAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version3.3.1Product Build 
Target VersionFixed in Version 
Summary0036688: Prepend make path versus append make path in Makefile
DescriptionEspecially on Windows, sometimes wrong tools (from cygwin,...) are picked up by the Makefile.
Most of these cases can be prevented by prepending the make directory in stead of appending.

This "fix" could be applied to trunk and fixes. And also for Lazarus trunk and fixes.

Additional InformationFix:

-SEARCHPATH+=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE))))
+SEARCHPATH:=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE)))) $(SEARCHPATH)
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files
  • fpcpatch_all_makesearchpath.patch (489 bytes)
    Index: Makefile
    ===================================================================
    --- Makefile	(revision 44068)
    +++ Makefile	(working copy)
    @@ -21,7 +21,7 @@
     SEARCHPATH:=$(subst ;, ,$(PATH))
     endif
     endif
    -SEARCHPATH+=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE))))
    +SEARCHPATH:=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE)))) $(SEARCHPATH)
     PWD:=$(strip $(wildcard $(addsuffix /pwd.exe,$(SEARCHPATH))))
     ifeq ($(PWD),)
     PWD:=$(strip $(wildcard $(addsuffix /pwd,$(SEARCHPATH))))
    

Activities

Alfred

2020-02-10 09:17

reporter  

fpcpatch_all_makesearchpath.patch (489 bytes)
Index: Makefile
===================================================================
--- Makefile	(revision 44068)
+++ Makefile	(working copy)
@@ -21,7 +21,7 @@
 SEARCHPATH:=$(subst ;, ,$(PATH))
 endif
 endif
-SEARCHPATH+=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE))))
+SEARCHPATH:=$(patsubst %/,%,$(subst \,/,$(dir $(MAKE)))) $(SEARCHPATH)
 PWD:=$(strip $(wildcard $(addsuffix /pwd.exe,$(SEARCHPATH))))
 ifeq ($(PWD),)
 PWD:=$(strip $(wildcard $(addsuffix /pwd,$(SEARCHPATH))))

Tomas Hajny

2020-02-10 09:55

manager   ~0120993

Just a question - wouldn't the described case most likely result in running a make from the cygwin installation and thus the added path would be that one of cygwin anyway? Extension of the search path is intended for making sure that the tool is found even though it is not included in PATH. I don't think that the makefile should try to override the settings of the user (even though I understand that some users may not be aware of settings on their own machine - however trying to be smarter than the user is a rather questionable practise from my point of view).

Alfred

2020-02-10 10:13

reporter   ~0120994

Naturally, if the proposed patch would have any negative side effect, it should not be applied.
It is intended at helping the user.

Example case from a user on Windows.
This user has a cygwin install. And has cygwin in his path.
He installs FPC from the standard source. The path towards FPC and its helping binaries (make, pwd, ...) is added into the path.
If he wants to build FPC himself (from official sources), the Makefile will use (some) cygwin tools, even if he uses the make utility from FPC itself.

Again, it's not about being smarter, its about helping.
But again, any negative effect can mean and has to be a no-go for this patch.

Marco van de Voort

2020-02-10 10:41

manager   ~0120995

I'm not sure this helps all cases. FPC make afaik also changes behaviour if it finds a sh.exe, which might be cygwin's.

Alfred

2020-02-10 10:53

reporter   ~0120996

@Marco
It definitely does not help in all cases. But it helps in some.

I tried replacing sh.exe with the COMPEC environment variable on Windows.
That works perfect (Win10) on win32, but fails on win64.
It would have been very nice if this (trick) would have worked in all cases.

Florian

2020-02-10 21:55

administrator   ~0121004

To me it sounds usefull and reasonable to force to use the utilities provided by FPC. After all, we tested them heavily. And: if make is from a certain dir, imo all other tools should come from this dir as well.

Personally, I have a script called fpc.bat which replaces all default pathes only by a few for FPC development because I were bitten too often by the fact that some random outdated cygwin/whatever util came into my way.

Marco van de Voort

2020-02-10 22:23

manager   ~0121006

I have no FPC, mingw or cygwin directories in the PATH, but have batchfiles like this (which are autogenerated btw). It saves the default path to OLDPATH, and e.g. the cygwin will then use that.

@echo off
Rem we save oldpath only once, so we can always revert to it.
if "%OLDPATH%" neq "" goto :nosave
set OLDPATH=%PATH%
:nosave
SET PATH=%OLDPATH%
PATH c:\pp32\bin\i386-win32;%PATH%

Tomas Hajny

2020-02-10 23:41

manager   ~0121007

That's actually the point from my point of view - if the user sets up something reflecting the situation on his own machine (personally, I have basically something similar as described by Florian and Marco except for the fact that my batch file allows defining the location of the starting FPC version as a parameter defined in another environment variable), I want to keep the control rather than having it decided by the makefile (since that is much more difficult to override). However take it just as my 2 cents, I don't think that the proposed change would result in major problems, especially if the behaviour is documented properly.

Mattias Gaertner

2020-02-20 10:16

manager   ~0121173

Note for Lazarus: The Makefiles are regenerated by fpcmake before release, so the patch won't survive.

Issue History

Date Modified Username Field Change
2020-02-10 09:17 Alfred New Issue
2020-02-10 09:17 Alfred File Added: fpcpatch_all_makesearchpath.patch
2020-02-10 09:55 Tomas Hajny Note Added: 0120993
2020-02-10 10:13 Alfred Note Added: 0120994
2020-02-10 10:41 Marco van de Voort Note Added: 0120995
2020-02-10 10:53 Alfred Note Added: 0120996
2020-02-10 21:55 Florian Note Added: 0121004
2020-02-10 22:23 Marco van de Voort Note Added: 0121006
2020-02-10 23:41 Tomas Hajny Note Added: 0121007
2020-02-20 10:16 Mattias Gaertner Note Added: 0121173