jperon / lyluatex

Alternative à lilypond-book pour lualatex
MIT License
56 stars 11 forks source link

Missing final system on recompile #290

Closed sudobash1 closed 2 years ago

sudobash1 commented 2 years ago

I have found that on the master branch of lyluatex, I often get an erroneous output if I compile my document twice. For a trivial example, I have two files in a directory:

test.tex:

\documentclass{minimal}

\usepackage{lyluatex}

\begin{document}
    \lilypondfile{test.ly}
\end{document}

test.ly:

melody = \relative c'' {
  g1 | \break
  g1 | \break
  g1 |
}

\score {
  \new Staff  <<
    \melody
  >>
}

The first time I compile it I get this result with all three systems:

image

But if I compile it again (without changing anything) I only get two systems:

image

No further recompilations give me any different results. I am stuck with only two systems. I have to remove the tmp-ly directory to get all three systems again.

I found that this bug does not seem to exist in the 1.0 version, so I used git bisect to try and find the guilty commit. It only seems to occur in the most recent commit (06a7c270ec2251d4ad6bcda016879689dd4e6484).

I have not had any time to delve in any deeper, or try to root cause it yet. This is my first time using lyluatex (I am new to lilypond in general), so I wasn't positive that this wasn't user error, but it seems pretty clearly erroneous to me.

I am running Ubuntu 20.04.2 LTS under WSL2 and using it's version of tex-live.

Cheers 🍻

eefweenink commented 2 years ago

Op 2 aug. 2022, om 23:41 heeft Stephen Robinson @.**@.>> het volgende geschreven:

melody = \relative c'' { g1 | \break g1 | \break g1 | }

\score { \new Staff << \melody

}

eefweenink commented 2 years ago

Dear Stephen,

I tested this several times. The code looks good to me. I cannot reproduce the error you have. So your testfiles are OK. You might have to dig into the logfiles, see what is happening on your system.

regards, Eef

Op 2 aug. 2022, om 23:41 heeft Stephen Robinson @.**@.>> het volgende geschreven:

I have found that on the master branch of lyluatex, I often get an erroneous output if I compile my document twice. For a trivial example, I have two files in a directory:

test.tex:

\documentclass{minimal}

\usepackage{lyluatex}

\begin{document} \lilypondfile{test.lyhttp://test.ly} \end{document}

test.lyhttp://test.ly:

melody = \relative c'' { g1 | \break g1 | \break g1 | }

\score { \new Staff << \melody

}

The first time I compile it I get this result with all three systems:

[image]https://user-images.githubusercontent.com/1923776/182477133-f9f65066-f0e7-4435-b5fc-057b89ff8ffc.png

But if I compile it again (without changing anything) I only get two systems:

[image]https://user-images.githubusercontent.com/1923776/182477264-00bc9f0b-59a7-427c-a6b7-3151154d8c1a.png

No further recompilations give me any different results. I am stuck with only two systems. I have to remove the tmp-ly directory to get all three systems again.

I found that this bug does not seem to exist in the 1.0 version, so I used git bisect to try and find the guilty commit. It only seems to occur in the most recent commit (06a7c27https://github.com/jperon/lyluatex/commit/06a7c270ec2251d4ad6bcda016879689dd4e6484).

I have not had any time to delve in any deeper, or try to root cause it yet. This is my first time using lyluatex (I am new to lilypond in general), so I wasn't positive that this wasn't user error, but it seems pretty clearly erroneous to me.

I am running Ubuntu 20.04.2 LTS under WSL2 and using it's version of tex-live.

Cheers 🍻

— Reply to this email directly, view it on GitHubhttps://github.com/jperon/lyluatex/issues/290, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOERR2LY2CLLDZWAAN4UW23VXGI2HANCNFSM55MUYAQQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

sudobash1 commented 2 years ago

@eefweenink Thanks for looking into this. I tried on some other computers that I have with different Linux distros. One replicated the issue, but one didn't. If I get some free time, I will dig deeper into this.

sudobash1 commented 2 years ago

I misspoke before. It turns out I can replicate the issue on all my systems. I dug in a bit with some log statements like this:

diff --git a/lyluatex.lua b/lyluatex.lua
index 96a14aa..5f1d25d 100644
--- a/lyluatex.lua
+++ b/lyluatex.lua
@@ -708,9 +708,11 @@ function Score:count_systems(force)
         for f in lfs.dir(self.tmpdir) do
             if f:match(systems) then
                 count = count + 1
+                info('have match: '..f..' count is now:'..count)
             end
         end
         if count > 1 then count = count - 1 end
+        info('final count: '..count)
         self.system_count = count
     end
     return self.system_count

The first time this ran, I saw this in the log:

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642.eps count is now:1

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-3.eps count is now:2

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-1.eps count is now:3

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-2.eps count is now:4

(lyluatex)      final count: 3

The second time I ran it, I saw this in the log:

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-3.eps count is now:1

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-1.eps count is now:2

(lyluatex)      have match: b5b3fdf67544dc397107028bf59b7642-2.eps count is now:3

(lyluatex)      final count: 2

So the first time I run it, it sees an extra .eps file, but subsequent runs have that count lowered. This seems to happen because Score:delete_intermediate_files() removes the first .eps file after the first run completes. So it gets counted in the first run only and not the second. The line with count = count - 1 in it seems to assume that it is still present.

One solution might be to move the call to delete_intermediate_files to before count_systems happens and remove the decrementing of count as it should no longer be needed.

The other solution I found was to change the lua pattern to only match files with -%d at the end (although I am not sure that this wouldn't break other use cases)

diff --git a/lyluatex.lua b/lyluatex.lua
index 96a14aa..89b5a25 100644
--- a/lyluatex.lua
+++ b/lyluatex.lua
@@ -704,13 +704,12 @@ end
 function Score:count_systems(force)
     local count = 0
     if force or not count then
-        local systems = self.output:match("[^/]*$").."%-?%d*%.eps"
+        local systems = self.output:match("[^/]*$").."%-%d+%.eps"
         for f in lfs.dir(self.tmpdir) do
             if f:match(systems) then
                 count = count + 1
             end
         end
-        if count > 1 then count = count - 1 end
         self.system_count = count
     end
     return self.system_count
jperon commented 2 years ago

@sudobash1 Thank you very much for reporting and investigating! I must confess I don’t have so much time as I used to have to closely follow all that.

Could you please test against #292 (branch fix-290) to ensure I didn’t miss anything?

Thanks again!

sudobash1 commented 2 years ago

@jperon I understand about not having as much time. I have children now, so I tend to be in the same boat (I'm using lyluatex to make a small hymnbook for them when I get spare moments). Thanks for being so responsive! I tried #292 and it is working well for me.

Cheers!