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

Redirection warning #1628

Closed
rpspringuel opened this issue Feb 17, 2025 · 16 comments
Closed

Redirection warning #1628

rpspringuel opened this issue Feb 17, 2025 · 16 comments
Labels

Comments

@rpspringuel
Copy link
Contributor

The warning about ignored arguments reared its head with Akira Kakuto in his testing and it bothered him enough that he emailed me a bug report (with a patch that removes the redirection). Perhaps we need to reassess just how "harmless" this warning is and see if there isn't something we can do about it while still leaving open the option of capturing the error it's meant to capture.

Refer to #1541 and #1606 for past discussion.

@rpspringuel
Copy link
Contributor Author

Quoting Akira's messages for reference:

I find a command line for gregorio is better if 2> %s is removed,
since message file is set by -l %s:

+++ gregoriotex.lua	Mon Feb 17 07:40:22 2025
@@ -1143,8 +1143,8 @@
    extra_args = extra_args..' -D'
  end

-  local cmd = string.format('%s %s -W -o %s -l %s "%s" 2> %s', gregorio_exe(),
-      extra_args, gtex_file, glog_file, gabc_file, glog_file)
+  local cmd = string.format('%s %s -W -o %s -l %s "%s"', gregorio_exe(),
+      extra_args, gtex_file, glog_file, gabc_file)
  res = os.execute(cmd)
  if res == nil then
    err("\nSomething went wrong when executing\n    '%s'.\n"

In the original case, unnecessary warnings:
Module gregoriotex Warning: ignored arguments: 2> tmp-gre\Populus...,
etc. are shown and written in tmp-gre/...glog file, at least on
Windows.
If the new patch is applied, no such warnings are shown.

Second message:

I believe that the above unnecessary warnings are shown in all
platforms in TeX Live.
In TeX Live, gregorio runs in restricted mode without
--shell-escape. In the restricted mode, all arguments are quoted.
Thus the redirection 2> foo becomes a usual argument.
I confirmed that if I execute like

lualatex --shell-escape main-lualatex.tex

the warnings are not shown. In this case, however, the final
argument of

local cmd = string.format('%s %s -W -o %s -l %s "%s" 2> %s', gregorio_exe(), extra_args, gtex_file, glog_file, gabc_file, glog_file)

should be changed, for example, like

local cmd = string.format('%s %s -W -o %s -l %s "%s" 2> %s', gregorio_exe(), extra_args, gtex_file, glog_file, gabc_file, "gerror.err")

because we have errors in tex/latex/base/ltluatex.lua in the
original case: doubled glog_file in -l %s and 2> %s.
I don't know why we have the errors in the original case.

@rpspringuel
Copy link
Contributor Author

Since I want to keep the ability to capture that kpsewhich error which started this whole thing, Akira's patch (which just reverts to what we had before) is the solution. We need something more complicated.

Perhaps we should only add the redirection argument when we discover we're not in restricted mode? I'm not sure how to detect that directly in Lua, but this StackExchange question provides an engine agnostic way of doing the detection which we could adapt to set an internal flag that Lua could read.

Akira also pointed out that our creativity in re-using the glog file for both this error redirection catch and the regular error logging that the executable does creates a problem for ltluatex.lua, so perhaps we need to simply create another file for catching this error.

rpspringuel added a commit to rpspringuel/gregorio that referenced this issue Feb 17, 2025
Since the redirection doesn't work in restricted mode, we only add it when `--shell-escape` has been given.  Further, rather than reusing the glog file, we create a special file for capturing the redirection error.  This addresses gregorio-project#1628.
@davidweichiang
Copy link
Contributor

davidweichiang commented Feb 18, 2025

What if gregorio prints to stdout until it successfully opens the log file? Then luatex can capture stdout (using io.popen).

@rpspringuel
Copy link
Contributor Author

I don't think that will work, the error we're trying to capture here isn't one that we try to write ourselves, but rather one which gregorio kind of "inherits" from the kpathsea libraries (it only appears when gregorio is compiled with those libraries). You're welcome to try and play with it, but my sense of this is that kpathsea is responsible for forcing this error to stderr, and that's why it bypasses our normal logging and requires this kind of slight of hand.

@davidweichiang
Copy link
Contributor

The error that (I thought) was in question is printed here:

gregorio/src/support.c

Lines 154 to 155 in ec7c886

gregorio_messagef("gregorio_out_name_ok", verbosity, __LINE__,
_("kpse prohibits write to file %s"), filename);

But I see that gregorio has an option to write the gtex file to stdout, so writing errors to stdout is not a good idea. (On the other other hand, there are several error messages, like the license and usage, that are printed to stdout already.)

@rpspringuel
Copy link
Contributor Author

I think that is the error (the C code is not my specialty), but you'll note that we're already using gregorio_messagef which should make use of our logging feature (it's what we use everywhere else for error logging, and those messages end up in the log file). @henryso tried to force the message into the log in his rewrites (merged in #1606), but was unable to do so. We came to the conclusion that kpse was forcing the message to stderr and thus we had to do this sort of redirection.

If you can find a way to get the message into the glog file without redirection, that would be great. I don't like the idea of having a log file specifically for this one error message, I just don't know how to do it otherwise at this point.

@davidweichiang
Copy link
Contributor

Here's what I see in the develop branch:

% ./gregorio-6_0_0 --version
Gregorio 6.0.0-develop-ba5c4323-4648 (kpathsea version 6.4.0).
  etc.
% ./gregorio-6_0_0 test.gabc -o ../test.gtex -l test.glog
% cat test.glog
error:kpse prohibits write to file ../test.gtex

So the original error in question does get redirected correctly. If we do the same thing again, we want it to append, not overwrite:

% ./gregorio-6_0_0 test.gabc -o ../test.gtex -l test.glog
% cat test.glog
error:kpse prohibits write to file ../test.gtex
error:kpse prohibits write to file ../test.gtex
%

That looks good again. The only error that has to go to stderr is of course if the glog file itself is not writable:

% ./gregorio-6_0_0 test.gabc -o ../test.gtex -l ../test.glog
error:kpse prohibits write to file ../test.glog

I believe it's in this one case that the new .glog.err file will be non-empty in shell-escape mode, and it's in this one case that the error would be lost in restricted \write18 mode.

My suggestion above was to send this one error to stdout so that the Lua code can capture it, but I no longer think that's a good idea and don't have a better idea. Maybe the Lua code can write the .glog.err file into the .glog file and then remove the .glog.err file.

@rpspringuel
Copy link
Contributor Author

That doesn't match what I recall, but it's been several years since we looked at this so it might be that my memory is mistaken.

@davidweichiang, have you checked what happens when a gregorio call that would trigger this error happens in a TeX document?

@davidweichiang
Copy link
Contributor

davidweichiang commented Feb 18, 2025

If I do the following, using branch develop,

  • /my/dir/tmp-gre/error-6_0_0.gtex exists and /my/dir/error.gabc is newer than it
  • cd /my/dir/subdir
  • lualatex ../error

Then the run pauses with an error, and after continuing, I see /my/dir/subdir/error-6_0_0.glog:

ignored arguments: 2> error-6_0_0.glog
Usage: gregorio [OPTION]... [-s | INPUT_FILE]
Try 'gregorio --help' for more information.
Proceeding anyway...
error:kpse prohibits write to file /my/dir/tmp-gre/error-6_0_0.gtex

I'm not seeing why the glog file is being created inside subdir, but other than that, this looks good.

@davidweichiang
Copy link
Contributor

It creates the glog inside subdir because the tex file just says \gregorioscore{error.gabc}, so it creates the glog file in the current directory, which is subdir.

@davidweichiang
Copy link
Contributor

Another small issue is that both the C and Lua code append to the glog file, and neither ever clears it. So in theory it will keep getting longer and longer.

@rpspringuel
Copy link
Contributor Author

@davidweichiang, based on your testing, it seems that it may no longer be possible for errors to end up in stderr and not our glog file. If that's the case, then we can get rid of the redirection altogether. Unfortunately I've lost my tester files for this error, so I can't quickly confirm this behavior on my end.

It creates the glog inside subdir because the tex file just says \gregorioscore{error.gabc}, so it creates the glog file in the current directory, which is subdir.

As I understand it, kpse requires files to be written to the current working directory (or subdirectories thereof). It prohibits writing to parent directories (or subdirectories thereof that aren't the current directory or subdirectories thereof). I would like the glog file to always end up in a tmp-gre folder even when someone's playing shenanigans with where they're calling luatex from. I'm not sure why the tmp-gre folder is not being created in your toy example.

Another small issue is that both the C and Lua code append to the glog file, and neither ever clears it. So in theory it will keep getting longer and longer.

The glog file should probably be regenerated whenever the gtex file is regenerated.

@davidweichiang
Copy link
Contributor

it seems that it may no longer be possible for errors to end up in stderr and not our glog file. If that's the case, then we can get rid of the redirection altogether.

Yes, that's how it looks to me, with the sole exception of not being able to open the glog file itself. This happened in commit 8ec1d64.

I'm not sure why the tmp-gre folder is not being created in your toy example.

Sorry I wasn't clear. The Lua code (function include_score) searches for an existing gtex file -- anywhere in TEXINPUTS -- and if it's not found (if not gtex_file) then it creates tmp-gre as desired.

But if a gtex file is found -- anywhere in TEXINPUTS -- the glog file is just in the same directory as the gabc file:

local glog_file = string.format("%s%s-%s.glog", file_dir, cleaned_filename,

It gets more complicated if the user does either lualatex path/to/file.tex or \gregorioscore{path/to/file.gabc}; I wasn't able to untangle all the cases.

The glog file should probably be regenerated whenever the gtex file is regenerated.

Agreed, and that should be a one-line fix.

error_file = freopen(error_file_name, "a", stderr);

@rpspringuel
Copy link
Contributor Author

Sorry I wasn't clear. The Lua code (function include_score) searches for an existing gtex file -- anywhere in TEXINPUTS -- and if it's not found (if not gtex_file) then it creates tmp-gre as desired.

But if a gtex file is found -- anywhere in TEXINPUTS -- the glog file is just in the same directory as the gabc file:

Ah! Now I get what's going on. This was to preserve backwards compatibility with files compiled before tmp-gre was added to the develop head: the files were headed to the same locations as they were before. Now that we're changing version numbers, this should no longer be an issue as the gtex file is tagged by version and an old version doesn't count as an existing gtex file that can be reused. We can probably simplify things in that area to force the tmp-gre creation and use for all glog files from now on.

I don't remember off-hand if there were other errors that went into the glog file (and thus necessitated the append mode). Was this stderr redirect the only thing that would get written to the glog file besides what the executable wrote? If so, then yes, we can revert to opening in write mode rather than append mode.

@davidweichiang
Copy link
Contributor

Was this stderr redirect the only thing that would get written to the glog file besides what the executable wrote?

Yes, a search for glog in the lua file doesn't turn up anything else.

@davidweichiang
Copy link
Contributor

PR #1632 tries to fix all of the above issues. The third commit is kind of big and I'll leave a few comments over there.

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

No branches or pull requests

2 participants