Hi, I'm used to test building LilyPond-master against upstream guilev2-versions, though I've to confess I limited testing to `make´ and `make doc´. Nevertheless, recently I tried `make test-baseline´ and it failed with _every_ guilev2, even with guile-2.0.14.
Then I went back to branch guile-v2-work and compiled it with
guile-2.0.14, which was guaranteed to work with the guilev2-patches in
this branch. And indeed it still works. This leads to the conclusion
the problem is in upstream LilyPond (at least for guile-2.0.14).
Thus I went up, patch by patch, and could identify two patches
preventing success for `make test-baseline´:
commit 96cdc755b547688c46097ba6a155aa1085ea7ac4
Author: David Kastrup <d...@gnu.org>
Date: Sun Feb 5 16:43:21 2017 +0100
Issue 5056/2: Don't assume uninstantiable engravers to be symbols
and
commit 51b6513eeeaea69293bd4f554f8021529ae85a49
Author: Dan Eble <nine.fierce.ball...@gmail.com>
Date: Mon Jul 2 13:36:48 2018 -0400
Issue 5366: Move warnings out of find/create context functions
The motivation for this is that Context::find_create_context () and
find_context_near () should probably be merged for maintainability,
but one of the differences between them that must be dealt with is
that find_create_context () logs when it fails and find_context_near
() does not. Adding warnings to find_context_near () risks being too
noisy, leaving the option taken here.
The new method Context::diagnostic_id (name, id) returns a formatted
string (e.g. "Voice" or "Voice = mel") for use in a log message. It
is used for the warnings that are being moved as well as a few other
existing warnings to increase consistency.
To verify, I checked out a new local branch out of
e57c27dc14a188bfdbcf0b1af9af0564218d9cdf
applied a bunch of patches for guilev2 (attached zip), reverted the
suspicious patches locally (attached as well) and compiled with
guile-2.0.14.
I've got a successful `make´ and `make test-baseline´. Didn't try
`make doc´ (worked already in a former test).
Some guessing:
Both reverted patches use
ly_scm_write_string
Maybe there's some problem there, similar to that of ly_scm2string,
fixed with Antonio's patch:
[PATCH 09/18] Fix ly_scm2string() to work with guile-2.0
(Find it in the attached zip as well)
Though, an attempt to fix ly_scm2string is out of my depth.
Furthermore, reverting said patches is not sufficient for a
successfull `make test-baseline´ with guile-2.2.x or guile-2.9.x.
More guessing:
Perhaps those guile-versions are more picky and a fix for
ly_scm_write_string would make them work as well, because
ly_scm_write_string is used at more than the two mentioned places.
Dan, David, I cc-ed you, because said patches are yours.
Thanks,
Harm
From d89461eea3da152b776d431569c5fae8a45552f9 Mon Sep 17 00:00:00 2001
From: Thomas Morley <thomasmorle...@gmail.com>
Date: Sat, 31 Aug 2019 11:36:56 +0200
Subject: [PATCH 183/183] Revert Issue 5056/2: Don't assume uninstantiable
engravers to be symbols
---
lily/translator-group.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lily/translator-group.cc b/lily/translator-group.cc
index e88cfebaf3..9d85c0a669 100644
--- a/lily/translator-group.cc
+++ b/lily/translator-group.cc
@@ -173,7 +173,8 @@ Translator_group::create_child_translator (SCM sev)
Translator *instance = unsmob<Translator> (trans);
if (!instance)
{
- warning (_f ("cannot find: `%s'", ly_scm_write_string (trans).c_str ()));
+// warning (_f ("cannot find: `%s'", ly_scm_write_string (trans).c_str ()));
+ warning (_f ("cannot find: `%s'", ly_symbol2string (scm_car (s)).c_str ()));
continue;
}
--
2.17.1
From 65eca7f55a2a808d17b8e2a27ac7a70e8c9e6e8d Mon Sep 17 00:00:00 2001
From: Thomas Morley <thomasmorle...@gmail.com>
Date: Sun, 1 Sep 2019 18:05:09 +0200
Subject: [PATCH] Revert "Issue 5366: Move warnings out of find/create context
functions"
This reverts commit 51b6513eeeaea69293bd4f554f8021529ae85a49.
---
.../regression/context-defaultchild-cycle.ly | 8 +--
.../regression/lyric-combine-empty-warning.ly | 4 +-
lily/change-iterator.cc | 7 +--
lily/context-def.cc | 8 +--
lily/context-specced-music-iterator.cc | 23 ++-------
lily/context.cc | 49 +++++++++----------
lily/include/context.hh | 9 ++--
lily/lyric-combine-music-iterator.cc | 28 +++++------
lily/quote-iterator.cc | 24 ++++-----
lily/simultaneous-music-iterator.cc | 32 +++++-------
10 files changed, 74 insertions(+), 118 deletions(-)
diff --git a/input/regression/context-defaultchild-cycle.ly b/input/regression/context-defaultchild-cycle.ly
index 46de604dfd..f62532f8bb 100644
--- a/input/regression/context-defaultchild-cycle.ly
+++ b/input/regression/context-defaultchild-cycle.ly
@@ -2,10 +2,10 @@
#(ly:set-option 'warning-as-error #t)
%% not sure why these warnings appear twice [dfe]
-#(ly:expect-warning (_ "default child context begins a cycle: ~a") 'Score)
-#(ly:expect-warning (_ "cannot find or create context: ~a") 'Bottom)
-#(ly:expect-warning (_ "default child context begins a cycle: ~a") 'Score)
-#(ly:expect-warning (_ "cannot find or create context: ~a") 'Bottom)
+#(ly:expect-warning (_ "default child context begins a cycle: `~a'") 'Score)
+#(ly:expect-warning (_ "cannot find or create new `~a'") 'Bottom)
+#(ly:expect-warning (_ "default child context begins a cycle: `~a'") 'Score)
+#(ly:expect-warning (_ "cannot find or create new `~a'") 'Bottom)
\header {
texidoc = "A @code{\\defaultchild} cycle does not induce an endless loop.
diff --git a/input/regression/lyric-combine-empty-warning.ly b/input/regression/lyric-combine-empty-warning.ly
index 5a83b77758..2f38988243 100644
--- a/input/regression/lyric-combine-empty-warning.ly
+++ b/input/regression/lyric-combine-empty-warning.ly
@@ -1,7 +1,7 @@
\version "2.19.2"
-#(ly:set-option 'warning-as-error #t)
-#(ly:expect-warning (ly:translate-cpp-warning-scheme "cannot find context: %s") "Voice = not-existing-notes")
+#(ly:set-option 'warning-as-error #f)
+#(ly:expect-warning (ly:translate-cpp-warning-scheme "cannot find Voice `%s'") "not-existing-notes")
\header {
diff --git a/lily/change-iterator.cc b/lily/change-iterator.cc
index aed72c50d9..c2040d7522 100644
--- a/lily/change-iterator.cc
+++ b/lily/change-iterator.cc
@@ -63,11 +63,8 @@ Change_iterator::change_to (Music_iterator &it,
ly_symbol2scm ("context"), dest->self_scm ());
}
else
- {
- Input *ori = it.get_music ()->origin ();
- ori->warning (_f ("cannot find context to change to: %s",
- Context::diagnostic_id (to_type, to_id).c_str ()));
- }
+ /* FIXME: constant error message. */
+ it.get_music ()->origin ()->warning (_ ("cannot find context to switch to"));
}
else if (it.get_outlet ()->is_alias (to_type))
{
diff --git a/lily/context-def.cc b/lily/context-def.cc
index e295191c9c..262a25d869 100644
--- a/lily/context-def.cc
+++ b/lily/context-def.cc
@@ -276,15 +276,15 @@ Context_def::internal_path_to_bottom_context (Output_def *odef,
Context_def *t = unsmob<Context_def> (find_context_def (odef, next_type_sym));
if (!is_instantiable (t))
{
- warning (_f ("cannot create default child context: %s",
- Context::diagnostic_id (next_type_sym, "").c_str ()));
+ warning (_f ("cannot create default child context: `%s'",
+ ly_symbol2string (next_type_sym).c_str ()));
return false;
}
if (std::find (path->begin (), path->end (), t) != path->end ())
{
- warning (_f ("default child context begins a cycle: %s",
- Context::diagnostic_id (next_type_sym, "").c_str ()));
+ warning (_f ("default child context begins a cycle: `%s'",
+ ly_symbol2string (next_type_sym).c_str ()));
return false;
}
diff --git a/lily/context-specced-music-iterator.cc b/lily/context-specced-music-iterator.cc
index 18c9be3a27..3164d6c330 100644
--- a/lily/context-specced-music-iterator.cc
+++ b/lily/context-specced-music-iterator.cc
@@ -18,12 +18,8 @@
*/
#include "music-wrapper-iterator.hh"
-
#include "context.hh"
-#include "input.hh"
-#include "international.hh"
#include "music.hh"
-#include "warn.hh"
class Context_specced_music_iterator : public Music_wrapper_iterator
{
@@ -46,24 +42,11 @@ Context_specced_music_iterator::construct_children ()
Context *a = 0;
if (to_boolean (get_music ()->get_property ("create-new")))
- {
- a = get_outlet ()->create_unique_context (ct, c_id, ops);
- if (!a)
- {
- Input *origin = get_music ()->origin ();
- origin->warning (_f ("cannot create context: %s",
- Context::diagnostic_id (ct, c_id).c_str ()));
- }
- }
+ a = get_outlet ()->create_unique_context (ct, c_id, ops);
else
{
- a = get_outlet ()->find_create_context (ct, c_id, ops);
- if (!a)
- {
- Input *origin = get_music ()->origin ();
- origin->warning (_f ("cannot find or create context: %s",
- Context::diagnostic_id (ct, c_id).c_str ()));
- }
+ a = get_outlet ()->find_create_context (get_music ()->origin (),
+ ct, c_id, ops);
}
// Q. Shouldn't descend-only block the creation of an unwanted context rather
diff --git a/lily/context.cc b/lily/context.cc
index 9f6ec8ebdb..db8c17b537 100644
--- a/lily/context.cc
+++ b/lily/context.cc
@@ -125,11 +125,18 @@ Context::create_unique_context (SCM name, const string &id, SCM operations)
Context *ret = 0;
if (daddy_context_ && !dynamic_cast<Global_context *> (daddy_context_))
ret = daddy_context_->create_unique_context (name, id, operations);
+ else
+ {
+ warning (_f ("cannot find or create new `%s'",
+ ly_symbol2string (name).c_str ()));
+ ret = 0;
+ }
return ret;
}
Context *
-Context::find_create_context (SCM n, const string &id,
+Context::find_create_context (Input *origin,
+ SCM n, const string &id,
SCM operations)
{
/*
@@ -141,7 +148,7 @@ Context::find_create_context (SCM n, const string &id,
if (gthis->get_score_context ())
{
return gthis->get_score_context ()->
- find_create_context (n, id, operations);
+ find_create_context (origin, n, id, operations);
}
}
@@ -161,7 +168,11 @@ Context::find_create_context (SCM n, const string &id,
Score context
*/
if (daddy_context_ && !dynamic_cast<Global_context *> (daddy_context_))
- return daddy_context_->find_create_context (n, id, operations);
+ return daddy_context_->find_create_context (origin, n, id, operations);
+
+ warning (_f ("cannot find or create `%s' called `%s'",
+ ly_symbol2string (n).c_str (), id),
+ origin);
return 0;
}
@@ -414,16 +425,14 @@ Context::get_default_interpreter (const string &id)
// It's interesting that this goes straight to creating a new hierarchy even
// if there might be an existing partial (or even full?) path to a bottom
// context. This deserves an explanation.
- SCM name = ly_symbol2scm ("Bottom");
- if (Context *c = create_unique_context (name, id, SCM_EOL))
- return c;
-
- // TODO: Avoiding a null return means the caller does not detect this
- // failure, so we have to log here if we want to log at all. It would be
- // more flexible to return null and let the caller decide whether to warn in
- // its situation. The caller might know a useful source location too.
- warning (_f ("cannot find or create context: %s",
- diagnostic_id (name, id).c_str ()));
+ if (Context *c =
+ create_unique_context (ly_symbol2scm ("Bottom"), id, SCM_EOL))
+ {
+ return c;
+ }
+
+ // We expect that create_unique_context () logged a warning.
+ // Remaining in this context is more graceful than returning null.
return this;
}
@@ -700,20 +709,6 @@ Context::context_name () const
return ly_symbol2string (context_name_symbol ());
}
-string
-Context::diagnostic_id (SCM name, const string& id)
-{
- // For robustness when this static method is called directly (e.g. after a
- // failure to create a context), we do not assume that name is a symbol.
- string result (ly_scm_write_string (name));
- if (!id.empty ())
- {
- result += " = ";
- result += id;
- }
- return result;
-}
-
Context *
Context::get_score_context () const
{
diff --git a/lily/include/context.hh b/lily/include/context.hh
index b57daa9de5..7d24982033 100644
--- a/lily/include/context.hh
+++ b/lily/include/context.hh
@@ -96,12 +96,7 @@ protected:
void unset_property_from_event (SCM);
public:
- // e.g. "mel" in "\context Voice = mel ..."
string id_string () const { return id_string_; }
-
- // formatted identification for log messages
- static string diagnostic_id (SCM name, const string& id);
-
SCM children_contexts () const { return context_list_; }
Dispatcher *event_source () const { return event_source_; }
@@ -157,7 +152,9 @@ public:
bool is_bottom_context () const;
bool is_removable () const;
- Context *find_create_context (SCM context_name, const string &id,
+ Context *find_create_context (Input *,
+ SCM context_name,
+ const string &id,
SCM ops);
Context *create_unique_context (SCM context_name, const string &context_id,
diff --git a/lily/lyric-combine-music-iterator.cc b/lily/lyric-combine-music-iterator.cc
index 2991da2531..1569e71546 100644
--- a/lily/lyric-combine-music-iterator.cc
+++ b/lily/lyric-combine-music-iterator.cc
@@ -330,23 +330,19 @@ Lyric_combine_music_iterator::process (Moment /* when */)
void
Lyric_combine_music_iterator::do_quit ()
{
- /* Don't print a warning for empty lyrics (in which case we don't try
- to find the proper voice, so it will not be found) */
- if (lyrics_found_ && !music_found_)
+ if (!music_found_)
{
- Music *m = get_music ();
-
- // ugh: defaults are repeated elsewhere
- SCM voice_type = m->get_property ("associated-context-type");
- if (!scm_is_symbol (voice_type))
- voice_type = ly_symbol2scm ("Voice");
-
- string id = robust_scm2string (m->get_property ("associated-context"),
- "");
-
- Input *origin = m->origin ();
- origin->warning (_f ("cannot find context: %s",
- Context::diagnostic_id (voice_type, id).c_str ()));
+ SCM voice_name = get_music ()->get_property ("associated-context");
+ SCM voice_type = get_music ()->get_property ("associated-context-type");
+ string name, type;
+ if (scm_is_string (voice_name))
+ name = ly_scm2string (voice_name);
+ type = robust_symbol2string (voice_type, "Voice");
+ /* Don't print a warning for empty lyrics (in which case we don't try
+ to find the proper voice, so it will not be found) */
+ if (lyrics_found_)
+ get_music ()->origin ()->warning (_f ("cannot find %s `%s'",
+ type.c_str (), name.c_str ()) + "\n");
}
if (lyric_iter_)
diff --git a/lily/quote-iterator.cc b/lily/quote-iterator.cc
index 1e2e5ad215..8ab43cf54f 100644
--- a/lily/quote-iterator.cc
+++ b/lily/quote-iterator.cc
@@ -125,25 +125,19 @@ Quote_iterator::construct_children ()
{
Music_wrapper_iterator::construct_children ();
- Context *cue_context = 0;
-
SCM name = get_music ()->get_property ("quoted-context-type");
+ SCM id = get_music ()->get_property ("quoted-context-id");
+
if (scm_is_symbol (name))
{
- SCM id = get_music ()->get_property ("quoted-context-id");
- std::string c_id = robust_scm2string (id, "");
- cue_context = get_outlet ()->find_create_context (name, c_id, SCM_EOL);
- if (!cue_context)
- {
- Input *origin = get_music ()->origin ();
- origin->warning (_f ("cannot find or create context: %s",
- Context::diagnostic_id (name, c_id).c_str ()));
- }
+ Context *cue_context =
+ get_outlet ()->find_create_context (get_music ()->origin (),
+ name, robust_scm2string (id, ""),
+ SCM_EOL);
+ quote_outlet_.set_context (cue_context);
}
-
- if (!cue_context)
- cue_context = get_outlet ()->get_default_interpreter ();
- quote_outlet_.set_context (cue_context);
+ else
+ quote_outlet_.set_context (get_outlet ()->get_default_interpreter ());
event_vector_ = get_music ()->get_property ("quoted-events");
diff --git a/lily/simultaneous-music-iterator.cc b/lily/simultaneous-music-iterator.cc
index 23ae944635..1dd11295b2 100644
--- a/lily/simultaneous-music-iterator.cc
+++ b/lily/simultaneous-music-iterator.cc
@@ -18,12 +18,10 @@
*/
#include "simultaneous-music-iterator.hh"
-
-#include "context.hh"
-#include "input.hh"
-#include "international.hh"
#include "music.hh"
+#include "context.hh"
#include "warn.hh"
+#include "context-def.hh"
Simultaneous_music_iterator::Simultaneous_music_iterator ()
{
@@ -60,21 +58,17 @@ Simultaneous_music_iterator::construct_children ()
SCM scm_iter = get_static_get_iterator (mus);
Music_iterator *mi = unsmob<Music_iterator> (scm_iter);
- Context *c = get_outlet ();
- if (j && create_separate_contexts_)
- {
- // create a new context of the same kind with the number as ID
- SCM name = ly_symbol2scm (c->context_name ().c_str ());
- string id = ::to_string (j);
- if (Context *other = c->find_create_context (name, id, SCM_EOL))
- c = other;
- else
- {
- Input *origin = get_music ()->origin ();
- origin->warning (_f ("cannot find or create context: %s",
- Context::diagnostic_id (name, id).c_str ()));
- }
- }
+ /* if create_separate_contexts_ is set, create a new context with the
+ number number as name */
+
+ SCM name = ly_symbol2scm (get_outlet ()->context_name ().c_str ());
+ Context *c = (j && create_separate_contexts_)
+ ? get_outlet ()->find_create_context (get_music ()->origin (),
+ name, ::to_string (j), SCM_EOL)
+ : get_outlet ();
+
+ if (!c)
+ c = get_outlet ();
mi->init_context (mus, c);
mi->construct_children ();
--
2.17.1
<<attachment: patches-for-guile-2-9-1.zip>>
