Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

\graphicspath' argument cannot contain macros anymore #1086

Closed
dbitouze opened this issue Jun 14, 2023 · 35 comments
Closed

\graphicspath' argument cannot contain macros anymore #1086

dbitouze opened this issue Jun 14, 2023 · 35 comments

Comments

@dbitouze
Copy link
Contributor

dbitouze commented Jun 14, 2023

Brief outline of the bug

In \graphicspath' argument, the paths could be specified via macros, as follows:

\newcommand{\imagesdir}{images}
\graphicspath{{\imagesdir/}}

This is no longer the case with an updated version of TeX Live 2023 (LaTeX2e <2023-06-01>):

! LaTeX Error: File `...' not found.

Minimal example showing the bug

When the following example:

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\documentclass{article}
\usepackage{graphicx}
\newcommand{\imagesdir}{images}
\graphicspath{%
  % {images/}%
  {\imagesdir/}%
}
\begin{document}
\includegraphics[width=.5\linewidth]{example-image-renamed}
\end{document}

is compiled with LaTeX2e <2023-06-01>, the following error occurs:

! LaTeX Error: File `example-image-renamed' not found.

In order to let you easily test this project, the structure of which being:

test-graphicspath
├── images
│   └── example-image-renamed.pdf
└── test-graphicspath.tex

here are an archive of it and an Overleaf clone of it which show that \graphicspath' argument containing macros, respectively:

  • doesn't work with recent version(s) of LaTeX,
  • did work with older versions of LaTeX (here LaTeX2e <2022-06-01> patch level 5).

Log file (required) and possibly PDF file

test-graphicspath.log

@muzimuzhi
Copy link
Contributor

Seems a l3kernel issue.

  • Before, For \IfFileExists{<file>}{<true code>}{<false code>}, if <file> is not found and \input@path is defined, \@iffileonpath{<file>}{<true code>}{<false code>} is used to test if <file> exists with one of path in \input@path prepended. In \@iffileonpath, \openin does a full expansion on each path in \input@path.
  • After the merge of Use expl3 file-exists test #1063, \IfFileExists switched to expl3 test \file_full_name:n. It's \__file_full_name_auxii:nn, one of the internals of \file_full_name:n that doesn't expand each path in \input@path.
\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\documentclass{article}
\usepackage{graphicx}

\makeatletter
\ExplSyntaxOn
\cs_gset:Npn \__file_full_name_auxii:nn #1 #2
  {
    \tl_if_blank:nTF {#2}
      {
        \seq_map_tokens:Nn \l_file_search_path_seq
          { \__file_full_name_aux:Nnn \seq_map_break:n {#1} }
        \cs_if_exist:NT \input@path
          {
            \tl_map_tokens:Nn \input@path
              { \__file_full_name_aux:Nne \tl_map_break:n {#1} } % <<< changed
          }
        \__file_name_end:
      }
      { \__file_ext_check:nn {#1} {#2} }
  }

\cs_generate_variant:Nn \__file_full_name_aux:Nnn {Nne}
\ExplSyntaxOff
\makeatother


\begin{document}
Explicit path

{
  \graphicspath{{images/}}
  \includegraphics[width=3cm]{example-image-renamed}
}

{
  \makeatletter
  \def\input@path{{images/}}
  \IfFileExists{example-image-renamed.pdf}TF
  [\UseName{file_full_name:n}{example-image-renamed.pdf}] % before
  \@iffileonpath{example-image-renamed.pdf}TF
  \makeatother
}

Macro as path

\newcommand{\imagesdir}{images}

{
  \graphicspath{{\imagesdir/}}
  \includegraphics[width=3cm]{example-image-renamed}
}

{
  \makeatletter
  \def\input@path{{\imagesdir/}}
  \IfFileExists{example-image-renamed.pdf}TF
  [\UseName{file_full_name:n}{example-image-renamed.pdf}] % before
  \@iffileonpath{example-image-renamed.pdf}TF
  \makeatother
}

\end{document}

image

@PhelypeOleinik
Copy link
Member

Yeah, \__file_full_name_aux:Nnn does \tl_to_str:n on the path, and the macro is turned into a string:

\cs_new:Npn \__file_full_name_aux:Nnn #1#2#3
  { \exp_args:Ne \__file_full_name_aux:nN { \tl_to_str:n {#3} / #2 } #1 }

I can't try now, but I can't see a reason for the \tl_to_str:n there. I think it should be \__file_name_expand:n, if we want to play safe, or nothing at all.

@muzimuzhi
Copy link
Contributor

I can't try now, but I can't see a reason for the \tl_to_str:n there.

From doc for \l_file_search_path_seq,

The entries are not expanded when used so may contain active characters but should not feature any variable content.

@PhelypeOleinik
Copy link
Member

From doc for \l_file_search_path_seq,

The entries are not expanded

Grrrrr

For \l_file_search_path_seq may be so, but \input@path should be expanded. Passing an extra argument for \__file_full_name_aux:Nnn, with \tl_to_str:n or \__file_name_expand:n, should do the trick

@muzimuzhi
Copy link
Contributor

Passing an extra argument for \__file_full_name_aux:Nnn, with \tl_to_str:n or \__file_name_expand:n, should do the trick

Like this?

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\documentclass{article}
\usepackage{graphicx}

\makeatletter
\ExplSyntaxOn
\cs_gset:Npn \__file_full_name_auxii:nn #1 #2
  {
    \tl_if_blank:nTF {#2}
      {
        \seq_map_tokens:Nn \l_file_search_path_seq
          { \__file_full_name_aux:NnNn \seq_map_break:n {#1} \tl_to_str:n }
        \cs_if_exist:NT \input@path
          {
            \tl_map_tokens:Nn \input@path
              {
                \__file_full_name_aux:NnNn
                  \tl_map_break:n {#1} \__file_name_expand:n
              }
          }
        \__file_name_end:
      }
      { \__file_ext_check:nn {#1} {#2} }
  }

\cs_new:Npn \__file_full_name_aux:NnNn #1#2#3#4
  {
    \exp_args:Ne \__file_full_name_aux:nN
      {
        \exp_args:Ne \__file_full_name_slash:n { #3 {#4} } #2
      }
      #1
  }
\ExplSyntaxOff
\makeatother


\begin{document}
Explicit path

{
  \graphicspath{{images/}}
  \includegraphics[width=3cm]{example-image-renamed}
}

{
  \makeatletter
  \def\input@path{{images/}}
  \IfFileExists{example-image-renamed.pdf}TF
  \beginunravel
  [\UseName{file_full_name:n}{example-image-renamed.pdf}]
  \endunravel
  \@iffileonpath{example-image-renamed.pdf}TF
  \makeatother
}

Macro as path

\newcommand{\imagesdir}{images}

{
  \graphicspath{{\imagesdir/}}
  \includegraphics[width=3cm]{example-image-renamed}
}

{
  \makeatletter
  \def\input@path{{\imagesdir/}}
  \IfFileExists{example-image-renamed.pdf}TF
  [\UseName{file_full_name:n}{example-image-renamed.pdf}]
  \@iffileonpath{example-image-renamed.pdf}TF
  \makeatother
}

\end{document}

Also the TeXhackers note for \l_file_search_path_seq should be updated, since they will be treated differently.

TeXhackers note: When working as a package in LaTeX2ε, expl3 will automatically append the current \input@path to the set of values from \l_file_search_path_seq.

@PhelypeOleinik
Copy link
Member

Like this?

Haven't tested, but looks good!

Though I do wonder about \l_file_search_path_seq not expanding... This is most of the time not what people want. Also, at the time active (UTF-8) chars were LaTeX2e-robust, but now they are \protected, so expanding is not a problem. I'm thinking a (formally) breaking change is better here. I'd like opinions from others

@davidcarlisle
Copy link
Member

Also, at the time active (UTF-8) chars were LaTeX2e-robust, but now they are \protected

I was just going to make the same observation. I think we could consider this

@muzimuzhi
Copy link
Contributor

The \tl_to_str:n and the corresponding doc for \l_file_search_path_seq were added in commit latex3/latex3@bf52c87 (Overhaul l3file functions, 2017-06-19).

@josephwright
Copy link
Member

Also, at the time active (UTF-8) chars were LaTeX2e-robust, but now they are \protected

I was just going to make the same observation. I think we could consider this

As @muzimuzhi notes, I added this at the same time as other str -> tl stuff, and UTF-8 support wasn't really the reason. I feel that file data is strings, so I'd not expect expansion of macros in the seq. Indeed, I wonder if it's even documented for \input@path or if that was arguably a mistake in the implementation.

@josephwright
Copy link
Member

I can see a stronger argument for expansion in \graphicpath/\input@path than the seq: in the latter, you have V-type expansion for adding material from tl storage, so the entries really should be 'proto-strings'.

@davidcarlisle
Copy link
Member

davidcarlisle commented Jun 14, 2023

, I wonder if it's even documented for \input@path

I'd say for any user documented command such as \input@path (or even more\graphicspath) expansion is expected unless explicitly documented otherwise. \input, \include, \includegraphics, \IfFileExists and friends all expand the filename argument, so it would be strange if the path was not treated the same way.

@josephwright
Copy link
Member

, I wonder if it's even documented for \input@path

I'd say for any user documented command such as \input@path (or even more\graphicspath) expansion is expected unless explicitly documented otherwise. \input, \include, \includegraphics, \IfFileExists and friends all expand the filename argument, so it would be strange if the path was not treated the same way.

OK, so the only question then is do others agree with me at the code level? A change is easy enough either way, so it's not a technical issue.

@davidcarlisle
Copy link
Member

OK, so the only question then is do others agree with me at the code level?

Not really:-) but it doesn't matter much at the L3 level, as, as you say, you can control expansion when adding to the sequence. So happy enough that the l3 sequence is a str so in practice would always be built using :V or :e or whatever argument types as having unexpanded macro names in a file path is never wanted.

@josephwright
Copy link
Member

OK, so the only question then is do others agree with me at the code level?

Not really:-) but it doesn't matter much at the L3 level, as, as you say, you can control expansion when adding to the sequence.

That's rather what I was getting at: typically, in expl3 expansion is more controlled. That doesn't mean it never happens, of course, but I'm not really seeing why you'd need to store a set of macros in a seq rather than putting the values of the macros in the seq.

@davidcarlisle
Copy link
Member

but I'm not really seeing why you'd need to store a set of macros in a seq rather than putting the values of the macros in the seq.

same as any use of macros, to gain a level of indirection. if \input@path is {\thischapter} then the path changes each chapter just changing some general chapter setting without needing to redefine the sequence.

I don't see macros that finally expand to stringified filenames any differently to macros expanding to typeset text so the "not really" answer is because I don't see that the fact the end use is a string has any bearing on whether expansion should be suppressed (well except when it has to as tex is tex)

@josephwright
Copy link
Member

@davidcarlisle I can see that there is a difference between a file name (where we can rely on x-type expansion being equivalent to built-in expansion of an n-type argument), and a seq (where adding a literal string isn't the same as keeping variability). What bothers me is that I worry it encourages what feels to me like problematic behaviours:

  • Something like \thechapter is typesetting information, and file system stuff should be decoupled from that (i.e. it might look the same but should not rely on programmatic mapping)
  • The file paths for any given 'class' of file should be global to a document,, i.e. there should be no risk of say a figure for Ch. 2 and Ch. 3 having the same file name. (So that if you need to move files around, send them to others, etc., they have unique names that cannot be confused.)

I realise I might be a bit risk-averse in file naming: comes from my day job I suspect.

@josephwright
Copy link
Member

Obviously, if the consensus is to change, I'll sort (I can do an expl3 release tomorrow whatever is decided)

@josephwright
Copy link
Member

I've thought about this and on balance I'd like to keep users happy, so I'll make the change in expl3. Should be sorted today.

@josephwright
Copy link
Member

@davidcarlisle Quick query. Should \input@path (etc.) support only macros inside groups, or do we also need to cover

\def\foo{{./bar/}{./baz}}
\def\input@path{\foo}

josephwright added a commit to latex3/latex3 that referenced this issue Jun 15, 2023
@josephwright josephwright self-assigned this Jun 15, 2023
@FrankMittelbach
Copy link
Member

to cover that ll you need is one \expandafter, isn't it? It either hits a { or it expands the \foo.

@josephwright
Copy link
Member

to cover that ll you need is one \expandafter, isn't it? It either hits a { or it expands the \foo.

I was thinking simple x-type expanding before the mapping, but the question is whether it's supposed to be supported.

@muzimuzhi
Copy link
Contributor

but the question is whether it's supposed to be supported.

Testing on overleaf's texlive 2022 (latex2e 2022-06-01 PL5) and on local texlive 2023 with rollback both show

\def\foo{{./bar/}{./baz}}
\def\input@path{\foo}

is not supported before.

@josephwright
Copy link
Member

I will have l3kernel with CTAN shortly: the fix is entirely there so will be sorted irrespective of any PL business.

@FrankMittelbach
Copy link
Member

but the question is whether it's supposed to be supported.

Testing on overleaf's texlive 2022 (latex2e 2022-06-01 PL5) and on local texlive 2023 with rollback both show

\def\foo{{./bar/}{./baz}}

\def\input@path{\foo}

is not supported before.

Good to know, but not that relevant here in my opinion. The example use case that David gave could easily be of that nature. And supporting it should be trivial. so I would do both.

@josephwright
Copy link
Member

but the question is whether it's supposed to be supported.

Testing on overleaf's texlive 2022 (latex2e 2022-06-01 PL5) and on local texlive 2023 with rollback both show

\def\foo{{./bar/}{./baz}}

\def\input@path{\foo}

is not supported before.

Good to know, but not that relevant here in my opinion. The example use case that David gave could easily be of that nature. And supporting it should be trivial. so I would do both.

But that's putting the internal detail of the implementation of a data structure into a macro and expecting it to 'jsut work'. It might feel reasonable for \input@path, but what then happens for the l3 seq?

@josephwright
Copy link
Member

but the question is whether it's supposed to be supported.

Testing on overleaf's texlive 2022 (latex2e 2022-06-01 PL5) and on local texlive 2023 with rollback both show

\def\foo{{./bar/}{./baz}}

\def\input@path{\foo}

is not supported before.

Good to know, but not that relevant here in my opinion. The example use case that David gave could easily be of that nature. And supporting it should be trivial. so I would do both.

They are very different changes conceptually: one is to not suppress expansion of values passed from an iteration, the other is to force expansion and hope it produces something that is then suitable for iteration at all.

@FrankMittelbach
Copy link
Member

But that's putting the internal detail of the implementation of a data structure into a macro and expecting it to 'jsut work'. It might feel reasonable for \input@path, but what then happens for the l3 seq?

I see no "seq" above :-)

In my opinion it should work for \input@path (both this use case as well as e-expanding the inner brace groups - as you did if I remember correctly) . Programming level expl is different than user level (and \input@path is historically user level).

@FrankMittelbach
Copy link
Member

They are very different changes conceptually: one is to not suppress expansion of values passed from an iteration, the other is to force expansion and hope it produces something that is then suitable for iteration at all.

sorry don't see the argument. \input@path either contains a set of brace groups representing directories or it doesn't. If it doesn't then either it is a \fooexpanding to such a set of brace groups or it is invalid input by the user and would fail anyway. To hit the content of \input@path therefore with an o-expansion means either you hit the brace and nothing happens or you hit the \foowhich either contains an anppropriate structure or it would die anyway. So where is the problem? I agree with you that "technically" these are two very different cases but conceptionally it is just allowing indirection.

@josephwright
Copy link
Member

sorry don't see the argument. \input@path either contains a set of brace groups representing directories or it doesn't. If it doesn't then either it is a \fooexpanding to such a set of brace groups or it is invalid input by the user and would fail anyway. To hit the content of \input@path therefore with an o-expansion means either you hit the brace and nothing happens or you hit the \foowhich either contains an anppropriate structure or it would die anyway. So where is the problem? I agree with you that "technically" these are two very different cases but conceptionally it is just allowing indirection.

Feels to me like the difference between allowing \def\foo{foo}\begin{\foo} and \def\foo{\begin{foo}} (or similar): we are happy with the first approach in general as the environment name is a string (and yes I know it can fail for verbatim-like cases), but I think we'd say the latter is not supported.

@josephwright
Copy link
Member

(I'll hold off on the l3kernel release to see if others have a view on this)

@josephwright
Copy link
Member

BTW, checking back the oldre mechanism does

\expandafter \@tfor \expandafter \reserved@b \expandafter :\expandafter =\input@path \do { ... }

so would only work as already observed with brace groups directly inside \input@path.

@josephwright
Copy link
Member

@davidcarlisle Thoughts?

@FrankMittelbach
Copy link
Member

BTW, checking back the oldre mechanism does

\expandafter \@tfor \expandafter \reserved@b \expandafter :\expandafter =\input@path \do { ... }

so would only work as already observed with brace groups directly inside \input@path.

I don't insist on making it more flexible if that is what it was for the last 30odd years

@josephwright
Copy link
Member

Code has gone to CTAN today

@davidcarlisle
Copy link
Member

so would only work as already observed with brace groups directly inside \input@path.
@davidcarlisle Thoughts?

sorry I know it's gone out already, but for the record I think that's OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants