Re: Issue 5588: fix color-profile warning in output-distance (issue 545100043 by nine.fierce.ball...@gmail.com)
LGTM
https://codereview.appspot.com/545100043/diff/545110043/scripts/build/output-distance.py
File scripts/build/output-distance.py (right):
https://codereview.appspot.com/545100043/diff/545110043/scripts/build/output-distance.py#newcode100
scripts/build/output-distance.py:100: system ('convert +profile icc
-depth 8 -crop %dx%d+0+0 %s %s/crop2.png' % (dims + (new, dir)))
What about using `-strip` instead of `+profile icc`?
https://codereview.appspot.com/545100043/
Issue 5588: fix color-profile warning in output-distance (issue 545100043 by nine.fierce.ball...@gmail.com)
Reviewers: ,
Description:
https://sourceforge.net/p/testlilyissues/issues/5588/
The warning is "RGB color space not permitted on grayscale PNG" and the
solution is to strip the profile from the image.
Please review this at https://codereview.appspot.com/545100043/
Affected files (+4, -2 lines):
M scripts/build/output-distance.py
Index: scripts/build/output-distance.py
diff --git a/scripts/build/output-distance.py
b/scripts/build/output-distance.py
index
395fa65a6d6cb11105efb502a861816565884df8..5cf1def9af4a09b245add9d0caa6cf0d6958bc7a
100755
--- a/scripts/build/output-distance.py
+++ b/scripts/build/output-distance.py
@@ -94,8 +94,10 @@ def compare_png_images (old, new, dest_dir):
min (dims1[1], dims2[1]))
dir = get_temp_dir ()
-system ('convert -depth 8 -crop %dx%d+0+0 %s %s/crop1.png' % (dims +
(old, dir)))
-system ('convert -depth 8 -crop %dx%d+0+0 %s %s/crop2.png' % (dims +
(new, dir)))
+# "+profile icc" strips the ICC profile, which suppresses the
+# warning "RGB color space not permitted on grayscale PNG."
+system ('convert +profile icc -depth 8
-crop %dx%d+0+0 %s %s/crop1.png' % (dims + (old, dir)))
+system ('convert +profile icc -depth 8
-crop %dx%d+0+0 %s %s/crop2.png' % (dims + (new, dir)))
system1 ('compare -depth 8 -dissimilarity-threshold
1 %(dir)s/crop1.png %(dir)s/crop2.png %(dir)s/diff.png' % locals ())
Re: [gnu.org #1443339] Re: Fyi: this list, lilypond-devel, just had it's subject [tag] and footer removed
"Ian Kelling via RT" writes: > On Thu Oct 24 22:56:44 2019, andrew.bern...@gmail.com wrote: >> See my post re this on the User list. >> >> Andrew > > Sorry, I'm short on time today, I would like to reply directly to > lilypond-user, if you could let them know that, perhaps the footer or > header setting for lilypond-user was just some whitespace that mailman > was ignoring, because I don't see any change either. Munging was > unnecessary in that case, so this is just a purely good change to stop > unnecessarily munging. I think there was a footer: lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel that is now gone. -- David Kastrup
Re: Fix 'check' target without tidy (issue 573150043 by jonas.hahnf...@gmail.com)
Thanks for this. Seeing a warning message for an optional program bothered me, but addressing it was out of my depth. What do you think about using "false" rather than "no" if the program is not found? That would be more defensive because if "$(TIDY) ..." is executed as "false ...", we can be pretty confident that it will fail consistently, but some developer might have a program called "no" in the path that would do something unexpected. https://codereview.appspot.com/573150043/
Re: Issue 5587: allow configure --enable-ubsan (issue 575180043 by nine.fierce.ball...@gmail.com)
On 2019/10/25 17:08:38, lemzwerg wrote: LGTM. Your changes are two commits, right? Right. aclocal.m4:274: # "print a verbose error report and exit the program" Why quotation marks in the comment? Heh. I'm quoting the GCC manual without citing it. https://codereview.appspot.com/575180043/
[gnu.org #1443339] Re: Fyi: this list, lilypond-devel, just had it's subject [tag] and footer removed
On Thu Oct 24 22:56:44 2019, andrew.bern...@gmail.com wrote: > See my post re this on the User list. > > Andrew Sorry, I'm short on time today, I would like to reply directly to lilypond-user, if you could let them know that, perhaps the footer or header setting for lilypond-user was just some whitespace that mailman was ignoring, because I don't see any change either. Munging was unnecessary in that case, so this is just a purely good change to stop unnecessarily munging. -- Ian Kelling | Senior Systems Administrator, Free Software Foundation GPG Key: B125 F60B 7B28 7FF6 A2B7 DF8F 170A F0E2 9542 95DF https://fsf.org | https://gnu.org
Re: doc: Update section on "commit access" (issue 566930043 by jonas.hahnf...@gmail.com)
LGTM https://codereview.appspot.com/566930043/
Re: Issue 5587: allow configure --enable-ubsan (issue 575180043 by nine.fierce.ball...@gmail.com)
LGTM. Your changes are two commits, right? https://codereview.appspot.com/575180043/diff/559180043/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/575180043/diff/559180043/aclocal.m4#newcode274 aclocal.m4:274: # "print a verbose error report and exit the program" Why quotation marks in the comment? https://codereview.appspot.com/575180043/
Re: doc: Update section on "commit access" (issue 566930043 by jonas.hahnf...@gmail.com)
Thanks for the corrections.
https://codereview.appspot.com/566930043/diff/551130043/Documentation/contributor/source-code.itexi
File Documentation/contributor/source-code.itexi (right):
https://codereview.appspot.com/566930043/diff/551130043/Documentation/contributor/source-code.itexi#newcode2217
Documentation/contributor/source-code.itexi:2217: @command{git config
@w{push.default} matching} command
This @w{} command is not necessary, since a full stop is not a
hyphen-like character.
https://codereview.appspot.com/566930043/
Fix 'check' target without tidy (issue 573150043 by jonas.hahnf...@gmail.com)
LGTM https://codereview.appspot.com/573150043/
Re: doc: Update section on "commit access" (issue 566930043 by jonas.hahnf...@gmail.com)
On 2019/10/24 19:05:49, lemzwerg wrote:
LGTM
https://codereview.appspot.com/566930043/diff/581200043/Documentation/contributor/source-code.itexi
File Documentation/contributor/source-code.itexi (right):
https://codereview.appspot.com/566930043/diff/581200043/Documentation/contributor/source-code.itexi#newcode2196
Documentation/contributor/source-code.itexi:2196: The
@command{git@tie{}remote@tie{}set-url} command above should modify
your
I think it's not necessary to use `@tie' here – `@command' uses a
different
font, so it's rather clear where the command starts and ends. It
might make
sense, however, to avoid a line break by using `@w{set-url}', making
it clear
that the hyphen has really to be typed.
I've removed all @tie for this section. There are more throughout this
document, but that's out of the scope for this patch.
https://codereview.appspot.com/566930043/
Issue 5587: allow configure --enable-ubsan (issue 575180043 by nine.fierce.ball...@gmail.com)
Reviewers: ,
Description:
https://sourceforge.net/p/testlilyissues/issues/5587/
The new configuration option --enable-ubsan instruments compiled code
with the Undefined Behavior Sanitizer (UBSan).
GCC UBSan options are documented here:
https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Instrumentation-Options.html
I ran the regression tests with UBSan enabled, and it found that a bool
value was being read as something other than 0 or 1. Initializing more
of the members of Constrained_breaking fixed that.
Please review this at https://codereview.appspot.com/575180043/
Affected files (+46, -14 lines):
M aclocal.m4
M lily/constrained-breaking.cc
M lily/include/constrained-breaking.hh
Index: aclocal.m4
diff --git a/aclocal.m4 b/aclocal.m4
index
beb042ca7bfa484477c97d0e73c2cffad171ebfd..b6452253168e099cdc105236814e2dc0b42e2325
100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -169,6 +169,7 @@ AC_DEFUN(STEPMAKE_COMPILE_BEFORE, [
profile_b=no
debug_b=yes
pipe_b=yes
+ubsan_b=no
AC_ARG_ENABLE(debugging,
[AS_HELP_STRING(
@@ -200,6 +201,12 @@ AC_DEFUN(STEPMAKE_COMPILE_BEFORE, [
[compile with -pipe. Default: on])],
[pipe_b=$enableval])
+AC_ARG_ENABLE(ubsan,
+[AS_HELP_STRING(
+[--enable-ubsan],
+[instrument with the Undefined Behavior Sanitizer. Default:
off])],
+[ubsan_b=$enableval])
+
if test "$optimise_b" = yes; then
OPTIMIZE=" -O2 -finline-functions"
# following two lines are compatibility while Patchy has not
@@ -248,8 +255,29 @@ AC_DEFUN(STEPMAKE_COMPILE, [
fi
fi
-CFLAGS="$CFLAGS $OPTIMIZE"
+# If UBSan requested, test if it works and add to CFLAGS.
+if test "$ubsan_b" = yes; then
+save_cflags="$CFLAGS"
+CFLAGS=" -fsanitize=undefined $CFLAGS";
+AC_CACHE_CHECK([whether compiler understands -fsanitize=undefined],
+[stepmake_cv_cflags_ubsan],
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[/* UBSan test */]])],
+[stepmake_cv_cflags_ubsan=yes],
+[stepmake_cv_cflags_ubsan=no]))
+CFLAGS=$save_cflags
+if test $stepmake_cv_cflags_ubsan = yes; then
+SANITIZE="$SANITIZE -fsanitize=undefined"
+fi
+fi
+
+if test -n "$SANITIZE"; then
+# "print a verbose error report and exit the program"
+SANITIZE="$SANITIZE -fno-sanitize-recover"
+fi
+
+CFLAGS="$CFLAGS $OPTIMIZE $SANITIZE"
CPPFLAGS=${CPPFLAGS-""}
+LDFLAGS="$LDFLAGS $SANITIZE"
AC_MSG_CHECKING([for IEEE-conformance compiler flags])
save_cflags="$CFLAGS"
@@ -277,7 +305,7 @@ AC_DEFUN(STEPMAKE_CXX, [
AC_PROG_CXX
STEPMAKE_OPTIONAL_REQUIRED(CXX, c++, $1)
-CXXFLAGS="$CXXFLAGS $OPTIMIZE"
+CXXFLAGS="$CXXFLAGS $OPTIMIZE $SANITIZE"
AC_SUBST(CXX)
AC_SUBST(CXXFLAGS)
Index: lily/constrained-breaking.cc
diff --git a/lily/constrained-breaking.cc b/lily/constrained-breaking.cc
index
6ec503b1206dc646f1e46a2f88d036c97acf8ad1..e7c05f64ec4afc7c53a4f4b81219fbf195e65677
100644
--- a/lily/constrained-breaking.cc
+++ b/lily/constrained-breaking.cc
@@ -350,18 +350,14 @@ Constrained_breaking::prepare_solution (vsize start,
vsize end, vsize sys_count)
Constrained_breaking::Constrained_breaking (Paper_score *ps)
{
- valid_systems_ = systems_ = 0;
start_.push_back (0);
- pscore_ = ps;
- initialize ();
+ initialize (ps);
}
Constrained_breaking::Constrained_breaking (Paper_score *ps, vector
const )
: start_ (start)
{
- valid_systems_ = systems_ = 0;
- pscore_ = ps;
- initialize ();
+ initialize (ps);
}
static SCM
@@ -377,13 +373,11 @@ min_permission (SCM perm1, SCM perm2)
/* find the forces for all possible lines and cache ragged_ and
ragged_right_ */
void
-Constrained_breaking::initialize ()
+Constrained_breaking::initialize (Paper_score *ps)
{
- if (!pscore_)
-return;
+ valid_systems_ = systems_ = 0;
+ pscore_ = ps;
- ragged_right_ = to_boolean (pscore_->layout ()->c_variable
("ragged-right"));
- ragged_last_ = to_boolean (pscore_->layout ()->c_variable
("ragged-last"));
system_system_space_ = 0;
system_markup_space_ = 0;
system_system_padding_ = 0;
@@ -393,6 +387,16 @@ Constrained_breaking::initialize ()
score_markup_padding_ = 0;
score_markup_min_distance_ = 0;
+ if (!pscore_)
+{
+ ragged_right_ = false;
+ ragged_last_ = false;
+ return;
+}
+
+ ragged_right_ = to_boolean (pscore_->layout ()->c_variable
("ragged-right"));
+ ragged_last_ = to_boolean (pscore_->layout ()->c_variable
("ragged-last"));
+
Output_def *l = pscore_->layout ();
SCM spacing_spec = l->c_variable ("system-system-spacing");
Index: lily/include/constrained-breaking.hh
diff --git a/lily/include/constrained-breaking.hh
b/lily/include/constrained-breaking.hh
index
b0f1e375ea531ea332d5d7ef9a0e519bf7d3c261..15c132828e522497d8bb43aa1acd4e451f760cbd
100644
---
PATCHES - Countdown for October 25th
Hello, Here is the current patch countdown list. The next countdown will be on October 27th. A quick synopsis of all patches currently in the review process can be found here: http://philholmes.net/lilypond/allura/ Push: 5580 Replace deprecated functions from string module - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5580 http://codereview.appspot.com/566920044 Countdown: 5584 Rehabilitate regtest workflow - Dan Eble https://sourceforge.net/p/testlilyissues/issues/5584 http://codereview.appspot.com/571030043 5581 Fix underline-markup to make multiple calls have nice output - Thomas Morley https://sourceforge.net/p/testlilyissues/issues/5581 http://codereview.appspot.com/559150043 Review: No patches in Review at this time. New: 5585 doc: Update section on "commit access" - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5585 http://codereview.appspot.com/566930043 *** Regards, James
