Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added separated debugscripts option and first steps so that 'set -eu' works #699

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Nov 18, 2015

This is a huge pull request with lots of things in it.

Its initial intent was #688
by adding a separated debugscripts option so that e.g.

rear --debugscripts 'ue +h -o pipefail' help

now works as an initial proof of concept that rear can be
made working with things like 'set -eu'.

Because my final goal behind #688
is to make rear working with things like 'set -ue -o pipefail' I like
to have as a precondition support for advanced debugging settings
built-in in rear (instead of manually modifying the scripts).

While I implemented the debugscripts option I tested how it works
with things like

rear --debugscripts 'xvue -o pipefail' help

and during this testing I fixed all what fails in the rear scripts.

Unfortunately because of this that pull request here is big and implements three things at once:

1.)
The new debugscripts option which is fully backward compatible because the behaviour of the other options does not change.

2.)
I fixed all what fails because of 'set -ue -o pipefail' for the "help" workflow as a very first step into that direction.

3.)
When I was working on a particular piece of code I also changed it as needed to be better in compliance with the "Coding Style" at
https://github.com/rear/rear/wiki/Coding-Style

If you agree with what I did here, I will continue to make rear working with 'set -ue -o pipefail' step by step also for other workflows.

In any case I would very much appreciate it if you carefully check what I changed.

I assume at least in some cases you may wonder about the code before and afterwards - in this case please ask me - I think there are several places in the code where it is not clear what is actually meant so that I could have made plain wrong changes because I failed to understand what the actual intent is of a particular code.

Many thanks in advance for your review!

long option --debugscripts that requires an argument
where the argument contains the flags to be 'set'
e.g. --debugscripts=xv results 'set -xv'.
and also the other long options.
Corrected the help-workflow synopsis because '--' is parsed
before the COMMAND (plus optional ARGS).
at https://github.com/rear/rear/wiki/Coding-Style
and make it working fail-safe even with "set -eu"
(only usr/sbin/rear works with "set -eu" - not the other scripts).
with 'set -eu' and - by the way - adapted the coding style
according to https://github.com/rear/rear/wiki/Coding-Style
running the help workflow without creating stuff in /tmp
so that now for the first time calling it e.g. as
  rear --debugscripts 'ue +h -o pipefail' help
seems to work well - at least for me and for now.
long option --debugscripts that requires an argument
where the argument contains the flags to be 'set'
e.g. --debugscripts=xv results 'set -xv'.
and also the other long options.
Corrected the help-workflow synopsis because '--' is parsed
before the COMMAND (plus optional ARGS).
at https://github.com/rear/rear/wiki/Coding-Style
and make it working fail-safe even with "set -eu"
(only usr/sbin/rear works with "set -eu" - not the other scripts).
with 'set -eu' and - by the way - adapted the coding style
according to https://github.com/rear/rear/wiki/Coding-Style
running the help workflow without creating stuff in /tmp
so that now for the first time calling it e.g. as
  rear --debugscripts 'ue +h -o pipefail' help
seems to work well - at least for me and for now.
…tings'

of https://github.com/jsmeix/rear
into add_optional_value_to_specify_D_command_line_option_settings
@jsmeix
Copy link
Member Author

jsmeix commented Nov 18, 2015

I forgot:

For me also e.g. "rear -d -D mkrescue" still works so that for me it does not seem to be totally broken what I did - nevertheless I appreciate a careful review.

# rear -d -D mkrescue
Relax-and-Recover 1.17.2 / Git
Using log file: /var/log/rear/rear-nelson.log
Creating disk layout
Creating root filesystem layout
Copying files and directories
Copying binaries and libraries
Copying kernel modules
Creating initramfs
Making ISO image
Wrote ISO image: /var/lib/rear/output/rear-nelson.iso (48M)
You should also rm -Rf /tmp/rear.6oUBR6qZbKp0XT5
# echo $?
0

@jsmeix
Copy link
Member Author

jsmeix commented Nov 18, 2015

Regarding make rear working with ''set -ue -o pipefail"
I created #700

to the Coding Style at https://github.com/rear/rear/wiki/Coding-Style
that reads "Text Layout Indentation with 4 blanks, not tabs".
@jsmeix
Copy link
Member Author

jsmeix commented Nov 19, 2015

FYI:

Right now I tested on a SLES12-SP1 KVM/Qemu virtual machine
with 2.5TB virtual harddisk using SUSE's special 'gpt_sync_mbr'
and SLES12-SP1 default btrfs and xfs for /home that

rear -d -D mkbackup

and

rear -d -D recover

still "just work" for me.

@gdha gdha added this to the Rear v1.18 milestone Nov 19, 2015
gdha added a commit that referenced this pull request Nov 19, 2015
…ommand_line_option_settings

Added separated debugscripts option and first steps so that 'set -eu' works
@gdha gdha merged commit 45b6baa into rear:master Nov 19, 2015
@jsmeix
Copy link
Member Author

jsmeix commented Nov 25, 2015

This particular pull request is done (i.e. "fixed").

@jsmeix jsmeix deleted the add_optional_value_to_specify_D_command_line_option_settings branch November 25, 2015 12:56
pcahyna added a commit to pcahyna/rear that referenced this pull request Sep 3, 2019
- instead of expanding the previous value to an empty element, which then
causes problems later (introduced in rear#699 to pacify set -ue in bash 3).

The fix is still compatible with bash 3 and set -ue.

XXX similar changes should be applied to other numerous array operations found
in this file.

Fixes rear#2220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants