shiblon / latex-makefile

A Makefile for LaTeX - drop it in, type make, and magic happens.
Other
185 stars 30 forks source link

Unescaped '/' in sed command #190

Closed llbit closed 6 years ago

llbit commented 6 years ago

I noticed that I got messages from sed while building a large document. I narrowed it down and found that it was caused by using a subdirectory. I made a minimal reproducible example for the error here: https://github.com/llbit/Multi-PDF

The error message I get is:

= sections/frontpage.tex --> sections/frontpage.d sections/frontpage.pdf.1st.make (0-1) =
sed: -e expression #1, char 22: unknown command: `f'

By using SHELL_DEBUG=1 I saw the following:

+ sed -e /^No file sections\\/frontpage\.aux\./d sections/frontpage.log
+ egrep ^(.*Rerun .*|No file sections/frontpage\.[^.]+\.|No file .+\.tex\.|LaTeX Warning: File.*)$
sed: -e expression #1, char 22: unknown command: `f'

It seems like the sed command might not be properly escaped.

I made the following changes in the Makefile to remove the sed error:

diff --git a/Makefile b/Makefile
index f5810ee..5d8d877 100644
--- a/Makefile
+++ b/Makefile
@@ -2445,7 +2445,7 @@ endef
 # $(call test-log-for-need-to-run,<source stem>)
 define test-log-for-need-to-run
 $(SED) \
--e '/^No file $(call escape-fname-regex,$1)\.aux\./d' \
+-e '/^No file $(subst /,\/,$1)\.aux\./d' \
 $1.log \
 | $(EGREP) '^(.*Rerun .*|No file $1\.[^.]+\.|No file .+\.tex\.|LaTeX Warning: File.*)$$' \
 | $(EGREP) -q -v '^(Package: rerunfilecheck.*Rerun checks for auxiliary files.*)$$'
shiblon commented 6 years ago

OK, that's interesting. I think the proper way to fix this is to change the escape-fname-regex to handle the separator. I'll look at it and see what I can find.

shiblon commented 6 years ago

OK, on line 355 of Makefile.in, we have this:

escape-fname-regex  = $(subst /,\\/,$(subst .,\\.,$1))

So it looks like it's at least trying to escape the forward slash character, as well as dots. What I find curious about this is the fact that it seems to work with one backslash, but not two. Can you try just changing the subst command in escape-fname-regex to have a single backslash (instead of replacing it with your subst call) and report your findings?

llbit commented 6 years ago

I changed the regex to the following and it seems to remove the sed error:

diff --git a/Makefile b/Makefile
index f5810ee..cf9d7ca 100644
--- a/Makefile
+++ b/Makefile
@@ -1001,7 +1001,7 @@ cleanse-filename  = $(subst .,_,$(subst /,__,$1))

 # Escape dots and forward slashes.
 # $(call escape-fname-regex,str)
-escape-fname-regex     = $(subst /,\\/,$(subst .,\\.,$1))
+escape-fname-regex     = $(subst /,\/,$(subst .,\\.,$1))

 # Test that a file exists
 # $(call test-exists,file)

I'm using GNU sed 4.2.2 on Ubuntu 16.04.3 LTS.

shiblon commented 6 years ago

OK, excellent. Sorry for the delay. I'm pushing a new version to head that has this change in it.

shiblon commented 6 years ago

I think commit 6e3a363a2c616720b8f051a716293ccbd376ad9b fixes this.