manugarg / pacparser

A library to parse proxy auto-config (PAC) files
http://pacparser.manugarg.com
GNU Lesser General Public License v3.0
506 stars 116 forks source link

spidermonkey makefile causes error in the clean target #109

Closed mzpqnxow closed 2 years ago

mzpqnxow commented 3 years ago

This is really not a major issue and you can safely ignore it if you're busy. I'll be sending an (unrelated) PR that includes a fix for this minor issue if you want to cherry pick it or take it wholesale. Otherwise you can close this out if you think fixing it is a waste of time :)

The issue is in the clean target of the spidermonkey Makefile. It is reproducible on Linux using make && make clean

u@host:~/projects/pacparser/src $ make clean
rm -f libpacparser.so libpacparser.so.1 libjs.a pacparser.o pactester pymod/pacparser_o_buildstamp jsapi_buildstamp
cd pymod && python setup.py clean --all
running clean
'build/lib.linux-x86_64-2.7' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-2.7' does not exist -- can't clean it
cd spidermonkey && "make" clean
make[1]: Entering directory '/home/u/projects/pacparser/src/spidermonkey'
rm -rf js-buildstamp
rm js/src/*.o
rm: cannot remove 'js/src/*.o': No such file or directory
make[1]: *** [Makefile:42: clean] Error 1
make[1]: Leaving directory '/home/u/projects/pacparser/src/spidermonkey'
make: *** [Makefile:151: clean] Error 2

The spidermonkey object files are actually in js/src/Linux_All_DBG.OBJ/ for a default build on Linux. That directory is determined dynamically in js/src/config.mk- which is not included when clean is the target, as seen at the top of src/spidermonkey/Makefile, where it's wrapped with an ifneq

ifneq ($(MAKECMDGOALS),clean)
  DEPTH = js/src
  include js/src/config.mk
endif

...

clean:
    rm -rf js-buildstamp
    rm js/src/*.o

I was going to just adjust the rm command to rm -rf js/src/*.OBJ but thought I would see if there was a regression if the ifneq was removed. It turns out that both the build and clean targets seem to work fine without the ifneq, so the clean target can be changed to remove $(OBJDIR):

DEPTH = js/src
include js/src/config.mk

jsapi: js-buildstamp
...

clean:
    rm -rf js-buildstamp
    rm -rf js/src/$(OBJDIR)

Sorry for filing this silly "bug", I don't mean to waste your time. I just figured I'm fixing it in a local branch so I might as well let you know

The PR I am sending in implements a "feature" that was rejected once before (#28) so I'm not sure you'll want to accept it. However, I needed it for a project anyway, so I thought I would PR it, with the worst case being you close it out with a "no thanks". That's OK with me as it will be documented for any users that want it in the future

Thanks!