> > My point was that your sentence suggests
> > - that several scripts use Config. Actually, only one does.
> I count three scripts that use Config (salsa, uscan, mk-origtargz).
Sorry. A year ago, grepping scripts was enough, but uscan is not
alone in lib/ anymore.
3 is still less than 25.
> Don't use an absolute path to bash.
Done.
Note that Config.pm uses '/bin/bash'.
Out of curiousity: I was once taught that not relying on $PATH,
aliases and so on was safer. Do you have a reference (the question
seems to vague for a web search)?
> Unsetting the environment represents a change in the interface between
> devscripts and people's config files. An example of what could break:
> if someone sets HOME in their running zsh to a subdirectory, and in
> that directory has a devscripts config that uses ~/ then unsetting HOME
> could mean that the devscripts config would use their main home
> directory instead of the subdirectory they wanted to use.
Confvar only unsets the variables matching the list (or regex)
expected by the devscript tool.
devscripts.conf(5) requires this behaviour.
Confvar does not modify HOME, or any unrelated setting.
> Checking the syntax of the config files represents a change in the
> interface between devscripts and people's config files; config files
> that contain valid variables at the start and syntax errors at the end
> would stop setting the valid variables from the start.
> Telling bash to exit on errors by passing the -e option to bash
> represents a change in the interface between devscripts and people's
> config files. It could cause later parts of the config files to
> silently stop working if they have a command near the start that does
> not succeed.
I agree that these are changes in behaviour introduced by Confvar.
They have been approved by a devscripts maintainer in messages 10 to
27 in the same bug log.
> In the perl code use q{}/qq{} instead of ''/"" for readability; this is
> so that you can use quoting characters in the shell code without
> escaping them with backslash characters, which reduce the readability.
As far as I understand:
qq{} allows ' instead of \' inside literal strings.
q{} allows " instead of \" inside interpolated strings.
None happens in Confvar for now.
This would only raise the barrier for people less fluent in perl like
me.
> Use the spawn function from Dpkg::IPC instead of backticks because they
> introduce an extra unnecessary shell process while spawn does not use
> the shell so there can be no quoting issues no matter what is passed.
Done. Thanks.
> Pass the config filenames as arguments to the generated bash script and
> load the files from those arguments instead of from quoted strings.
> This avoids any possible mismatch between bash/Perl quoting code.
Done. Thanks.
> Use NUL characters (\0) to split the output up, this allows all
> characters to be represented instead of relying on Perl code to parse
> the output of the bash set command.
I could argue that, in order to see that your implementation is safer,
one has to be fluent with perl constructs like
map {qq{"\$$_"}} @key_names
and understand why " \0 \n are passed correctly via
printf '%s\0' "$a"
After many attempts at comparable solutions, I have encountered
scripts accepting a non-static range of variable identifiers defined
by a pattern, or even worst by the content of another user variable. I
have found no way to do this without parsing the output of the 'set'
built-in. The escaping used by 'set' is described in the comments at
the end of Confvar.pm.
For now, only the a=$'...' syntax is ignored (and bash array
variables, but this does not matter here). Bash uses it when the
variable contains control characters like \r \n. It would be possible
to also decode such values (by calling printf or similar), but
investing work in this direction does not seem in line with the
orientation selected by the maintainers in first messages of this bug
log.
> One enhancement I should have added to the Config module but didn't
> think of until now is to ensure that the stderr isn't parsed and that
As far as I understand, Dpkg::IPC::spawn separates stdout from stderr.
> any output the devscripts config generates also isn't parsed by the
> Perl code. Probably the way to do this is to print a few NUL characters
> in a row to indicate the start of the exported variables and discard
> any output before those NUL characters. Of course the devscripts config
> could print the NULs too and thus break the parsing, but outputting
> NULs from one's devscripts config seems unlikely to exist today.
In the messages quoted above, the maintainers are in favor of
requiring only simple affectations in configuration files, and even
removing shell support in the future if ever possible.
It would be way safer to report any unexpected output, and only go on
proceeding with an explicit override (for example if
lib/Devscripts/Output.pm::die_on_error is false).
> > It seemed right to answer a bug requesting help in the bug log,
> > but let us try a merge request instead.
> > https://salsa.debian.org/debian/devscripts/merge_requests/162
> This seems like a few separate issues all on one branch, usually they
> would be separate merge requests. I'd also squash the new code plus
> fixes to and perltidy of the new code into one commit.
Done. Thanks.
