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

TL 2017 pretest not finding executable #1362

Closed
rpspringuel opened this issue May 19, 2017 · 35 comments
Closed

TL 2017 pretest not finding executable #1362

rpspringuel opened this issue May 19, 2017 · 35 comments

Comments

@rpspringuel
Copy link
Contributor

I've just run the first pass of the test repository against the TL2017 pretest and it seems that the TeX/Lua code can't find the executable. All the dump and gtex tests pass, so I know the problem isn't with the executable itself. I'll try to make some time to work on this further this weekend, but if some one else has the time and inclination to look at this first, please feel free.

@rpspringuel
Copy link
Contributor Author

I'm kind of stumped at the moment. The following test document works as expected:

\documentclass{article}
\begin{document}
\directlua {
 real_gregorio_exe = 'gregorio'
 exe_version = io.popen(real_gregorio_exe..' --version', 'r')
 if exe_version then
   exe_version = exe_version:read("*line")
 end
 if exe_version == nil then
     tex.print("Woe is me!")
 else
     tex.print(exe_version)
 end}
\end{document}

Producing a pdf with the first line of the gregorio --version command. Misspell gregorio in the first line of Lua code and you'll get a document with "Woe is me!" instead.

However, in the context of gregoriotex.lua, exe_version ends up being nil (I can see this by adding a log function to print its value to the log).

@henryso
Copy link
Contributor

henryso commented May 20, 2017

If there's no difference with respect to --shell-escape on both runs, could it be that luatex (now) adds on more protection when something is loaded as a module?

@rpspringuel
Copy link
Contributor Author

Not as far as I know and there doesn't appear to be anything to that effect in the release notes.

Still, give the way things are I'm not willing to rule it out at this point.

@henryso
Copy link
Contributor

henryso commented May 20, 2017

I'm trying to install the pretest to work on the test project issue, but I'm having some trouble. With some luck, maybe I can help figure this out.

@henryso
Copy link
Contributor

henryso commented May 20, 2017

My finding is that the io.popen generates this message:

sh: all: command not found

@henryso henryso closed this as completed May 20, 2017
@henryso henryso reopened this May 20, 2017
@henryso
Copy link
Contributor

henryso commented May 20, 2017

(Sorry, I hit the "Close and comment" button by accident)

@rpspringuel
Copy link
Contributor Author

That's odd. It's almost like the subshell created by io.popen isn't getting the correct PATH variable. But if that's the case, why wouldn't my dinky test document have the same problem?

@henryso
Copy link
Contributor

henryso commented May 20, 2017

It very much seems like a bug. os.execute seems to work fine (but of course doesn't do what we need here).

@rpspringuel
Copy link
Contributor Author

I just tried a module variation on the test document:

\documentclass{article}
\usepackage{luatexbase}
\RequireLuaModule{test}
\begin{document}
\directlua {
 real_gregorio_exe = 'gregorio'
 exe_version = io.popen(real_gregorio_exe..' --version', 'r')
 if exe_version then
   exe_version = exe_version:read("*line")
 end
 if exe_version == nil then
     tex.print("Woe is me!")
 else
     tex.print(exe_version)
 end}

\directlua{test_me()}
\end{document}

With test.lua reading:

local function gregorio_exe()
  real_gregorio_exe = 'gregorio'
  exe_version = io.popen(real_gregorio_exe..' --version', 'r')
  if exe_version then
    exe_version = exe_version:read("*line")
  end
  if exe_version == nil then
    tex.print("Woe is me!")
  else
    tex.print(exe_version)
  end
end

function test_me()
  gregorio_exe()
end

This still works just fine for me. I think instead of trying to build up to the problem, I may need to cut down to it by starting with gregoriotex.lua modifying it to make the test result simple to see and then stripping out irrelevancies.

@henryso
Copy link
Contributor

henryso commented May 20, 2017

I am going to hold off on the gregorio-test fix until this one is solved, because version detection may change because of it.

@rpspringuel
Copy link
Contributor Author

Okay, this is weird. In order to establish a base line file against which I could perform the deconstruction, I made the following document:

\documentclass{article}

\usepackage{gregoriotex}

\begin{document}
Test
\gabcsnippet{(c3) test(g)}
\gregorioscore[a]{test.gabc}
\end{document}

with test.gabc being:

name: test;

%%

(c3) test(f)

This document compiles just fine.

Confused, I then ran ./gregorio-test.sh test/tex-output/PopulusSion/PopulusSion.tex to confirm that the error still existed and go the same sort of results I've been getting.

I then navigated into the the output tree of gregorio-test and manually compiled tex-output/PopulusSion from the command line. This compilation worked no problem.

It would thus seem that the problem is in how ./gregorio-test.sh invokes lualatex. Possibly not invoking a shell with the complete PATH variable?

@henryso
Copy link
Contributor

henryso commented May 21, 2017

Seems like a mess, because my tests (which resulted in all: command not found) did not involve gregorio-test. I was compiling files from the command line and getting that error. However, I was running latexmk from another shell, and I was not playing with PATH from the other shell.

@henryso
Copy link
Contributor

henryso commented May 22, 2017

Not sure this helps, but I created a command called "all" which echoes its arguments to a file. Apparently, these popen calls are executing "all commands are permitted".

@henryso
Copy link
Contributor

henryso commented May 22, 2017

I think popen is just broken in TL2017 pre-test. If you hardcode local real_gregorio_exe = 'gregorio' (instead of = nil) and then create a tex file with \gabcsnippet, it will fail, straight up, lualatex texfile.tex, from the command line.

@henryso
Copy link
Contributor

henryso commented May 22, 2017

Should we work around this popen bug by rewriting the relevant code in gregoriotex.lua to use os.execute and temporary files?

@rpspringuel
Copy link
Contributor Author

While I agree that something fishy is happening with io.popen it's not simply broken since all my simple documents work just fine. The trouble is figuring out the exact conditions necessary to produce the faulty behavior so that we can make a reasonable bug report.

In the meantime, I think working around it via os.execute and temporary files is a reasonable solution to getting GregorioTeX into a fully working state for TL2017.

@henryso
Copy link
Contributor

henryso commented May 22, 2017

I'm a little confused where I should make this change. I was going to put it in release-5.0, but I notice there are some README file changes there which are not in master. Is that the correct branch?

@rpspringuel
Copy link
Contributor Author

I'd put it in release-5.0. That branch will get merged into master when 5.0.2 is released. As we now have a bugfix, that will be soon. I didn't feel like the documentation changes were sufficient reason to make a new release, hence that branch not being identical to master currently.

henryso added a commit to henryso/gregorio that referenced this issue May 23, 2017
henryso added a commit to henryso/gregorio-test that referenced this issue May 24, 2017
rpspringuel added a commit to rpspringuel/gregorio that referenced this issue May 24, 2017
* release-5.0:
  Update Version number
  Added a change log linking to gregorio-project#1362.
  Changed from os.popen to os.execute due to TL2017 strangeness. Fixes gregorio-project#1362.
@rpspringuel
Copy link
Contributor Author

I'm reopening this issue because even though the work around eliminates it for us, we still haven't isolated and reported the actual luatex bug.

Along those lines, I've been working with the TL2017 pretest and the test repository and have manually compiled every test which fails as unable to create images when I run gregorio-test.sh with a simple (no flags) lualatex call (or luatex for the plain-tex tests). All have compiled without error. Thinking it might be a flag issue, I've tried one of the tests with each flag of the six flags that we use individually, again, no error. I tried with all six flags too (basically reproducing the call as gregorio-test.sh should be issuing it) and still had a successful compilation.

Frankly, I'm stumped. I cannot seem to reproduce the error without running gregorio-test.sh. @henryso, you indicated that you could. Can you provide the document you used to reproduce the error outside of gregorio-test.sh?

@henryso
Copy link
Contributor

henryso commented May 30, 2017

\documentclass[12pt,letterpaper]{memoir}
\usepackage{gregoriotex}
\usepackage{fontspec}
\setlrmarginsandblock{20mm}{20mm}{*}
\setulmarginsandblock{20mm}{20mm}{*}
\pagestyle{plain}
\checkandfixthelayout
\setmainfont[
    Extension = .otf ,
    UprightFont = *-Regular ,
    UprightFeatures = { SmallCapsFont = *SC-Regular } ,
    BoldFont = *-Bold ,
    BoldFeatures = { SmallCapsFont = *SC-Bold } ,
    ItalicFont = *-Italic ,
    ItalicFeatures = { SmallCapsFont = *SC-Italic } ,
    BoldItalicFont = *-BoldItalic ,
    BoldItalicFeatures = { SmallCapsFont = *SC-BoldItalic } ,
    Ligatures = TeX
]{Alegreya}
\renewcommand\bf\bfseries
\newcommand{\scoretitle}[1]{\begin{center}\begin{huge}\textsc{#1}\end{huge}\end{center}}
\gresetheadercapture{name}{scoretitle}{}
\gresetheadercapture{initial-lines}{gresetinitiallines}{}
\begin{document}
\grechangedim{afterinitialshift}{2.2mm}{scalable}
\grechangedim{beforeinitialshift}{2.2mm}{scalable}
\gresetinitiallines{1}
\grechangestyle{firstsyllable}{\scshape\relax}
\grechangestyle{firstsyllableinitial}{\MakeUppercase}
\gresetlinecolor{gregoriocolor}
\gabcsnippet{(c4) c(c)}

test

\end{document}

@rpspringuel
Copy link
Contributor Author

That document compiles cleanly for me. Were you doing anything special with the command line call?

@henryso
Copy link
Contributor

henryso commented May 30, 2017

No, I didn't, so it seems we were chasing different problems. I guess this popen problem is OS-specific as apparently Akira didn't see it under Windows.

@rpspringuel
Copy link
Contributor Author

My testing with the pretest has been on Linux 16.04, not Mac. What OS are you working with?

@rpspringuel rpspringuel reopened this May 30, 2017
@henryso
Copy link
Contributor

henryso commented May 30, 2017

I use Gentoo, but I am using the precompiled pre-test. I trimmed comments from my document before posting it. I'll try re-running tonight and let you know my findings.

@henryso
Copy link
Contributor

henryso commented May 30, 2017

Everything compiles cleanly for me now, using the old, popen-based gregoriotex.lua, so I can no longer reproduce the problem. The only change in my system since I last ran this test was to reboot it (actually, my UPS battery died and there was a power outage).

@rpspringuel
Copy link
Contributor Author

I've done some double checking of my work and it seems to be related to --shell-escape. A dirt simple document (below) compiles fine with lualatex test.tex but the error crops up with lualatex --shell-escape test.tex. Can you confirm this on your system @henryso?

\documentclass{article}
\usepackage{gregoriotex}
\begin{document}
\gabcsnippet{(c4) c(c)}

test

\end{document}

@henryso
Copy link
Contributor

henryso commented May 31, 2017

Yes, that fails for me as well, it tries to execute "all commands are permitted".

@rpspringuel
Copy link
Contributor Author

Okay then, we've eliminated the test repository from the chain of possible causes. Now to try and get the bulk of gregoriotex out of the way. Unfortunately that will probably have to wait for the weekend, I started a summer class this morning.

@henryso
Copy link
Contributor

henryso commented May 31, 2017

Ok, I have something:

\documentclass{article}
\begin{document}
\directlua{
    f = io.popen('gregorio --version', 'r')
    if f then
        l = f:read('*l')
        if l then
            tex.print('got '..l)
        else
            tex.print('l is nil')
        end
    else
        tex.print('f is nil')
    end
}
\end{document}

When run with --shell-escape produces l is nil and when run without --shell-escape produces got Gregorio 5.0.1 (kpathsea version 6.2.3).

@rpspringuel
Copy link
Contributor Author

Confirmed. Looks like we have a MWE for a bug report.

@rpspringuel
Copy link
Contributor Author

I'm trying to confirm the bug on both Mac and Windows now, but have to redownload the pretest in order to do so, as I seem to have deleted it after installing it on my Unbuntu 16.04 VM.

@henryso
Copy link
Contributor

henryso commented Nov 23, 2017

Were you able to confirm the bug? Is it still there?

@rpspringuel
Copy link
Contributor Author

The bug was confirmed, reported and fixed but not in time for TL2017. We should probably make a note to undo #1363 for TL2018, but I think we need to leave the work around in place for this year.

@rpspringuel
Copy link
Contributor Author

TL2018 pretest time. Someone should test to see if undoing this workaround is now feasible.

@rpspringuel rpspringuel added this to the 5.1.1 milestone Mar 15, 2018
@rpspringuel rpspringuel removed this from the 5.1.1 milestone Mar 25, 2018
@rpspringuel
Copy link
Contributor Author

I've tested this and undoing the workaround is possible. I'll open a PR with the code changes so that we can discuss them.

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

No branches or pull requests

2 participants