Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1196)

Issue 557640051: Issue #1204: Document, and add regtest for, external fonts.

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 1 week ago by Valentin Villenave
Modified:
2 months, 3 weeks ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Issue #1204: Document, and add regtest for, external fonts. Add a few details in NR 1.8.3.1 "Fonts explained", and a regtest with base64-embedded minimal OpenType fonts. (I didn’t want to use musical symbols since the whole point is that these glyphs are not to be confused with whatever’s installed on the OS.) The regtest has been tested with all major versions since 2.12, and current master with and without Guile2.

Patch Set 1 #

Total comments: 4

Patch Set 2 : Some nits from Werner #

Patch Set 3 : Use mkstemp! instead of tmpnam; don’t leave any temporary file behind. #

Patch Set 4 : Temporarily disable font-export-dir (fixes make test) #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+283 lines, -2 lines) Patch
M Documentation/notation/text.itely View 1 1 chunk +29 lines, -2 lines 0 comments Download
A input/regression/font-name-add-files.ly View 1 2 3 1 chunk +254 lines, -0 lines 1 comment Download

Messages

Total messages: 11
lemzwerg
Nice idea, thanks. Some nits and questions. https://codereview.appspot.com/557640051/diff/571940043/Documentation/notation/text.itely File Documentation/notation/text.itely (right): https://codereview.appspot.com/557640051/diff/571940043/Documentation/notation/text.itely#newcode1552 Documentation/notation/text.itely:1552: (and thus ...
3 months, 1 week ago (2020-03-30 16:06:08 UTC) #1
Valentin Villenave
On 2020/03/30 16:06:08, lemzwerg wrote: > Is it guaranteed that we can create this directory? ...
3 months, 1 week ago (2020-03-30 18:29:37 UTC) #2
Valentin Villenave
Some nits from Werner
3 months, 1 week ago (2020-03-30 18:31:11 UTC) #3
thomasmorley651
On 2020/03/30 18:29:37, Valentin Villenave wrote: > On 2020/03/30 16:06:08, lemzwerg wrote: > > Is ...
3 months, 1 week ago (2020-03-30 18:56:27 UTC) #4
Valentin Villenave
On 2020/03/30 18:56:27, thomasmorley651 wrote: > tmpnam is deprecated in guile-3.0.2 (sigh) Nice catch! > ...
3 months, 1 week ago (2020-03-30 19:56:55 UTC) #5
Valentin Villenave
Use mkstemp! instead of tmpnam; don’t leave any temporary file behind.
3 months, 1 week ago (2020-04-03 09:19:44 UTC) #6
Valentin Villenave
Temporarily disable font-export-dir (fixes make test)
3 months, 1 week ago (2020-04-03 11:23:14 UTC) #7
Valentin Villenave
https://codereview.appspot.com/557640051/diff/565870043/input/regression/font-name-add-files.ly File input/regression/font-name-add-files.ly (right): https://codereview.appspot.com/557640051/diff/565870043/input/regression/font-name-add-files.ly#newcode244 input/regression/font-name-add-files.ly:244: #(rmdir dummyfontdir) So, David ran into a "Directory not ...
3 months ago (2020-04-11 09:44:26 UTC) #8
Valentin Villenave
On 2020/04/11 09:44:26, Valentin Villenave wrote: > What could be the cause? So, pushed as ...
2 months, 3 weeks ago (2020-04-19 15:54:32 UTC) #9
dak
v.villenave@gmail.com writes: > On 2020/04/11 09:44:26, Valentin Villenave wrote: >> What could be the cause? ...
2 months, 3 weeks ago (2020-04-19 16:53:28 UTC) #10
Valentin Villenave
2 months, 3 weeks ago (2020-04-19 17:18:32 UTC) #11
On 4/19/20, David Kastrup <dak@gnu.org> wrote:
> v.villenave@gmail.com writes:
> Breaks my patchy again.  Whose patchy was comfortable moving it from
> staging to master?

Well, at any rate James’ patchy what fine with it. (As it went through
the review/countdown process not just once but twice.)

The regtest actually produces an additional PDF file that should allow
you to check manually that all temporary files have been removed.

Evidently in your case something else (but what?) gets created inside
the temporary directory, which it then can’t remove because it doesn’t
know about it. (I could issue a system call for `rm -rf`, but that
would miss the point.

I already caught one unintended side-effect which was that during make
doc, 'font-export-dir gets enabled and thus my temporary font file
gets referenced which broke make check. There may very well be another
side effect specific to either your system or your build process, but
I’d need help identifying it as I can’t seem to reproduce it myself
(obviously do feel free to revert in the meantime).

Cheers,
V.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b