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

Issue 345160043: relocate.cc: Introduce new command `set?'.

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 3 months ago by lemzwerg
Modified:
5 years, 3 months ago
Reviewers:
dak, Dan Eble
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

relocate.cc: Introduce new command `set?'. Contrary to `set', `set?' won't override environment variables that are already set. We need this for reproducable builds, in particular with gub: The standard fontconfig setup includes font files like `~/.fonts' or `/usr/share', which obviously must not be used. Up to now, the `fontconfig.reloc' file in gub uses the `set' relocation command, which doesn't allow gub to replace the fontconfig configuration file with a temporary alternative while building test files or documentation. Future changes in gub will use `set?' instead.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Typo. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M lily/relocate.cc View 1 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 6
dak
https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc File lily/relocate.cc (right): https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc#newcode376 lily/relocate.cc:376: if (command == "set?") Where/how is that being used? ...
5 years, 3 months ago (2018-12-21 17:00:50 UTC) #1
lemzwerg
> Where/how is that being used? Wouldn't this want documenting? It's nowhere documented, which pretty ...
5 years, 3 months ago (2018-12-21 17:47:26 UTC) #2
Dan Eble
https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc File lily/relocate.cc (right): https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc#newcode376 lily/relocate.cc:376: if (command == "set?") not "else if"?
5 years, 3 months ago (2018-12-22 00:56:05 UTC) #3
lemzwerg
Typo.
5 years, 3 months ago (2018-12-22 09:40:18 UTC) #4
lemzwerg
> not "else if"? Fixed, thanks.
5 years, 3 months ago (2018-12-22 09:41:37 UTC) #5
lemzwerg
5 years, 3 months ago (2018-12-23 21:01:23 UTC) #6
> This feature gets only activated if you configure with
> `--enable-relocation` (which you normally won't do).

I have to correct myself: The configure option `--enable-relocation` is dead
code that does nothing.  LilyPond's `--relocate` command line option is useless,
too: Since 12(!) years, relocation is always activated, without a possibility to
switch it off.

I will prepare a patch to clean this up.
Sign in to reply to this message.

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