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

Missing final system on recompile #290

Closed
sudobash1 opened this issue Aug 2, 2022 · 6 comments · Fixed by #292
Closed

Missing final system on recompile #290

sudobash1 opened this issue Aug 2, 2022 · 6 comments · Fixed by #292

Comments

@sudobash1
Copy link

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 (06a7c27).

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
Copy link

eefweenink commented Aug 3, 2022 via email

@eefweenink
Copy link

eefweenink commented Aug 3, 2022 via email

@sudobash1
Copy link
Author

@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
Copy link
Author

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 added a commit that referenced this issue Aug 5, 2022
jperon added a commit that referenced this issue Aug 5, 2022
@jperon
Copy link
Owner

jperon commented Aug 5, 2022

@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
Copy link
Author

@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!

jperon added a commit that referenced this issue Aug 5, 2022
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

Successfully merging a pull request may close this issue.

3 participants