#2627 PR `closed`: Enable a clean log file without suppressing stderr unconditionally¶

Labels: discuss / RFC, won't fix / can't fix / obsolete

OliverO2 opened issue at 2021-06-07 21:00:¶

This PR addresses concerns in https://github.com/rear/rear/issues/2623#issuecomment-855943435, just without throwing the baby out with the bath water.

When used with ReaR's default configuration, the only stderr messages remaining in the log are these (each preceded by one regular log line):

2021-06-07 22:44:07.949146931 Including prep/default/400_save_directories.sh
stat: cannot stat '/run/user/121/gvfs': Permission denied
stat: cannot stat '/run/user/7002/gvfs': Permission denied
stat: cannot stat '/run/user/7002/doc': Permission denied

2021-06-07 22:44:09.975175176 Saving filesystem layout (using the findmnt command).
WARNING: No blkio weight support
WARNING: No blkio weight_device support

2021-06-07 22:44:11.123708262 Including layout/save/default/445_guess_bootloader.sh
4+0 records in
4+0 records out
2048 bytes (2.0 kB, 2.0 KiB) copied, 6.7896e-05 s, 30.2 MB/s

2021-06-07 22:44:11.859673492 Including rescue/GNU/Linux/320_inet6.sh
fe80000000000000a1177f5f3e0b767b 05 40 20 80  bridge0
00000000000000000000000000000001 01 80 10 80       lo

2021-06-07 22:44:12.914842576 Files being excluded: /home/oliver/Repositories/open-source/rear/var/lib/rear/output/* dev/.udev dev/shm dev/shm/* dev/oracleasm dev/mapper dev/shm/* /etc/pki/tls/private /etc/pki/CA/private /etc/pki/nssdb/key*.db /usr/lib/ssl/private /usr/share/misc/magic /lib/udev/rules.d/66-snapd-autoimport.rules
tar: Removing leading `/' from member names
tar: Removing leading `/' from hard link targets

2021-06-07 22:44:25.892832876 Testing 'ldd /bin/bash' to ensure 'ldd' works for the subsequent 'ldd' tests within the recovery system
    linux-vdso.so.1 (0x00007fff401a2000)
    libtinfo.so.6 => /lib/x86_64-linux-gnu/libtinfo.so.6 (0x00007f2f158ed000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f2f158e7000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f2f156f5000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f2f15a4e000)

2021-06-07 22:44:32.813354972 Creating md5sums for regular files in /tmp/rear.TjE89ak2ljRqq4y/rootfs
/tmp/rear.TjE89ak2ljRqq4y/rootfs /home/oliver/Repositories/open-source/rear
/home/oliver/Repositories/open-source/rear

2021-06-07 22:45:17.189352792 Including output/ISO/Linux-i386/700_create_efibootimg.sh
3+0 records in
3+0 records out
100663296 bytes (101 MB, 96 MiB) copied, 0.0484193 s, 2.1 GB/s

2021-06-07 22:45:17.527583107 Including ISO UEFI boot (as triggered by USING_UEFI_BOOTLOADER=1)
xorriso 1.5.2 : RockRidge filesystem manipulator, libburnia project.

Drive current: -outdev 'stdio:/home/oliver/Repositories/open-source/rear/var/lib/rear/output/rear-juliet.iso'
Media current: stdio file, overwriteable
Media status : is blank
Media summary: 0 sessions, 0 data blocks, 0 data, 83.6g free
xorriso : WARNING : -volid text does not comply to ISO 9660 / ECMA 119 rules
Added to ISO image: directory '/'='/tmp/rear.TjE89ak2ljRqq4y/tmp/isofs'
xorriso : UPDATE :      31 files added in 1 seconds
xorriso : UPDATE :      31 files added in 1 seconds
xorriso : WARNING : Boot image load size exceeds 65535 blocks of 512 bytes. Will record 0 in El Torito to extend ESP to end-of-medium.
xorriso : UPDATE :  24.48% done
ISO image produced: 265653 sectors
Written to medium : 265653 sectors at LBA 0
Writing to 'stdio:/home/oliver/Repositories/open-source/rear/var/lib/rear/output/rear-juliet.iso' completed successfully.

2021-06-07 22:45:22.151615205 rear,148575 usr/sbin/rear -v mkrescue
  `-rear,164333 usr/sbin/rear -v mkrescue
      `-pstree,164334 -Aplau 148575
/home/oliver/Repositories/open-source/rear/usr/share/rear/lib/_input-output-functions.sh: line 151: kill: (164337) - No such process

In my view, all of these should not raise concerns even for the casual user, with the following exceptions, which indicate that something should be fixed:

ReaR should probably not mess with the /run temporary file system: stat: cannot stat '/run/user/121/gvfs': Permission denied stat: cannot stat '/run/user/7002/gvfs': Permission denied stat: cannot stat '/run/user/7002/doc': Permission denied
A process supposed to be killed doesn't exist: /home/oliver/Repositories/open-source/rear/usr/share/rear/lib/_input-output-functions.sh: line 151: kill: (164337) - No such process

I have checked other type invocations throughout ReaR. They all use either -p, -P, or -t (all of which do not produce stderr messages), or they redirect stderr to /dev/null.

jsmeix commented at 2021-06-08 12:56:¶

@OliverO2
on a quick first glance I do not like your proposed changes here
because as far as I understand it they basically fully contradict
my intentions behind
https://github.com/rear/rear/issues/2416 and
https://github.com/rear/rear/pull/2498
and go back to the state before - as far as I currently see - i.e.
back to misleading false alarm messages by default in the log
and as a consequence special coding at each and every code place
where some program might output some false alarm message on stderr.

I try to have a closer look as time permits but possibly it gets delayed until eternity
because I do not want to spend any more minute of my precious time with general
issues what ReaR outputs where under what conditions in which specific way
because I think there is no right solution for all users so such issues go on and on
and re-appear again and again ad infinitum and ad nauseam and in the end
it only wastes energy and health of all involved people.

@gdha @rmetrich @pcahyna and all @rear/contributors
I would much appreciate it if you find some time and have a closer look here.
Feel free to proceed as you like and as you think it is best for ReaR.

In general (except exceptions) I will not further deal with such kind of issues.
I may - as time permits - further explain my intentions behind
https://github.com/rear/rear/issues/2416 and
https://github.com/rear/rear/pull/2498
if things are not yet sufficiently clear to help you to decide what to do.

OliverO2 commented at 2021-06-08 13:13:¶

@jsmeix I have invested considerable time into this PR (it took a lot of searching). This was an attempt to address your valid concerns but it feels like the road is blocked. In addition, unfortunately, I see no empathy to also address my concerns. Sorry to see that we could not find a more constructive path here.

jsmeix commented at 2021-06-09 10:53:¶

@OliverO2
I think you misunderstood what I meant.

I want to make it possible to move this issue forward in a constructive way.
Therefore I moved myself out of the way to not block anything here.
Because I have a contradicting opinion compared to yours I leave it to others to decide.
I did not set me as assignee or reviewer here so others are free to do what they think is best.
I wrote "on a quick first glance / as far as I understand / as I currently see" to indicate
that what I wrote is not a thorougly thought decision but only my first impression
so others who decide should not consider it as some authoritative judgement.

Regarding "no empathy":
I think my refusing wording was misunderstood as "no empathy" but I did not mean that.
I only mean that I refuse to spend more time with those kind of issues.
I think you cannot imagine how much time I wasted in more than 20 years
(hundreds of hours worktime that cost me thousands of hours lifetime)
with various kind of "usability" or "user experience" issues (almost none related to ReaR)
where there was no right solution for all users so such issues went on and on
and re-appeared again and again ad infinitum and ad nauseam and in the end
they only wasted lots of energy and health of all involved people.
So to procet myself and my health I decided some years ago
to no longer waste time with usability/UX issues.
This does not mean I completely ignore usability/UX.
I intend to provide reasonably well usability/UX.
But I do no longer waste time with "only usability/UX" issues.
When usability/UX is really bad the root cause is basically always
a real bug and real bugs get fixed.

Regarding "also address my concerns":
I think what went wrong here is that this pull request is an implementation
but what should have had happened before is mutual understanding
of your concerns and my concerns.
I.e. we have now an implementation (that you like but I do not like)
but not yet a common understanding of what the actual problem is.

Regarding what the actual problem is:
I think the actual problem is that when ReaR errors out in non-debug modes
it only shows the ReaR error message but no message from the failed program
so it is hard to understand what the root cause is why ReaR did error out.

If my understanding of the actual problem is right
then what is needed is that when ReaR errors out in non-debug modes
it shows the ReaR error message plus messages from the failed program
to make it easier to understand what the root cause is why ReaR did error out.

So what is needed is not to have stderr in any case in the log but
what is needed is to get messages from the failed program
if and only if ReaR errors out.

If this can be implemented it should fix the specific actual problem here
and it does not show possibly false alarm messages when ReaR does not error out.

My totally untested off the top of my head idea
(the idea came to me during last night so sleeping over an issue always helps)
is based on

COMMAND ... 2>>"$RUNTIME_LOGFILE" || Error "..."

cf. https://github.com/rear/rear/issues/2416#issuecomment-855949302
which shows possibly false alarm messages when ReaR does not error out
that might be further enhanced to work somehow like

# function Error () { cat /tmp/current.stderr ; echo "Error: $@" ; }

# grep -Q welcome /etc/issue 2>/tmp/current.stderr || Error "grep failed with $?"
grep: invalid option -- 'Q'
Usage: grep [OPTION]... PATTERN [FILE]...
Try 'grep --help' for more information.
Error: grep failed with 2

# grep -q welcome /etc/issue 2>/tmp/current.stderr || Error "grep failed with $?"
Error: grep failed with 1

# grep -i welcome /etc/issue 2>/tmp/current.stderr || Error "grep failed with $?"
Welcome to \S - Kernel \r (\l).

It cannot be used as is in ReaR because in debug modes stderr must append to the log
but the above writes stderr into another file so I need to think more about it...
I wished something like

COMMAND ... 2>SPECIAL_FILE || Error "..."

where SPECIAL_FILE appends to the log in debug modes
and in non-debug modes is is a normal file where the Error function could extract things.

Perhaps we should redirect in non-debug modes strderr in general
into a separated normal file where the Error function could extract things
so in non-debug modes the ReaR log cannot contain possibly false alarm messages
but in non-debug modes stderr of all programs is still available for the Error function
to extract some latest stderr messages.

Because stderr messages could contain sensitive information
cf. https://github.com/rear/rear/issues/2416#issuecomment-855960979
that normal file would have to be deleted in any case when ReaR finishes
regardless how ReaR finishes i.e. also when ReaR errors out
because that normal file would contain all stderr messages of all programs
and not only the last ones of the failed program that let ReaR error out.

OliverO2 commented at 2021-06-10 22:03:¶

@jsmeix
Thank you for taking the time to answer. Briefly, because it's slightly late:

I think it would be difficult to go ahead with ReaR without your backing since you are the one who takes care of most issues.
Yes, I might not fully understand what your motivation was as all I had was your opinion as stated. I could not find detailed examples about which stderr messages were specifically annoying you. Regarding wasted hours on usability issues: It is possible that I can relate, having had similar experiences (most people have those in one way or the other). Without concrete examples I cannot know.
What I perceived as "no empathy" was not a rejection as such, but a rejection without addressing my arguments. When there is a conflict, I see no problem when people decide to not agree (hey, we are grown-ups). The problem is when it appears that one party does not address, but seemingly ignores, the other party's stated arguments. I gave concrete examples for my concerns, and a proof of concept to address yours. So when I say, for example, I cannot focus properly when rear -d mkrescue dumps lots of output lines at me, this is apparently important to me and should deserve a reaction.
With respect to the actual problem: Erroring out without giving a precise cause is one side of the issue. The other (and larger side) is errors which ReaR does not catch and which would compromise the result (i.e. produce a non-working rescue system). There is no way to foresee the possible conditions. A quick glance over the logfile (if it contains complete stderr output) is a way to safeguard against this. Separating stderr into an additional log file would break the association between logged ReaR actions and stderr output. It would also not really address your concerns, because when it's there, people will look into it.

It does not look likely that ReaR is on a path where I could continue to rely on it the way I need to. So I have decided to create an internal fork just for my use cases based on commit 2433c72f. I have already heavily modified the code to simplify it and increase usability (at least for me). The downside is that I can no longer track upstream developments. But in the end, this helps me to better allocate resources. Hopefully, it also helps you to proceed with what you find necessary to support ReaR in the future.

gdha commented at 2021-06-11 09:43:¶

@OliverO2 Disputes are normal and very human, however, for this alone leaving the project is hard and we cannot and will not stop you of doing that. You will fix your problems in the short run, but over a couple of months when we release a new major version of ReaR you may regret it. We try to find a way in reasonable solutions and/or fixes that does not break anything and we know even then we break our code sometimes. As you said there are only a few key contributors for the moment (like @jsmeix and @pcahyna who are trying to get the code to work under all circumstances after years of adding new features without seeing the bigger picture).
For that reason alone I'm in favor of a grand cleanup and trimming unneeded features - by way of speaking go back to the core.
That being said @OliverO2 it is with sad feelings that we see you leaving the upstream project, but we welcome you back anytime with pleasure.

OliverO2 commented at 2021-06-11 20:58:¶

@gdha Thank you for your kind words. Let me assure you that my decision is not a knee-jerk reaction to that dispute. As you say, disputes are normal (and even healthy), we have had those before, and it is not something in general to worry about. This one went a bit astray but not necessarily to the extent that we could not have resolved it in one way or another.

My concern in this case is the limited time I have available, and the characteristics of changes I needed to use rear safely and confidently. While unused code in plugins was never an issue for me (it just sits there), usability was not good enough. The required changes deeply affect existing ReaR code, and contradict what I perceived as the prevalent philosophy guiding ReaR at this time.

I am aware that this has a downside, cutting me off from bug fixes and added functionality I might find useful. But doing so anyway also shows my confidence that much of ReaR (at least the way I am using it) has become pretty mature over time and does not require lots of maintenance.

Anyway, contributing for a couple of years has been a pleasure and ReaR has mostly been a pretty welcoming project with respect to contributions.

Regarding your thoughts on cleanup, yes, consolidation is always a good idea as it makes the code base more manageable. I cannot say much about plugins as they did not really affect me (they might affect you when dealing with issues and testing releases, though). What I would consider in addition is to remove duplicate functionality, such as two sets of Btrfs code, two methods for disk sizing, and possibly others.

gdha commented at 2021-07-02 07:07:¶

@OliverO2 @jsmeix Do not get it wrong that we close this PR as we see no outcome on an acceptable PR by all involved parties.
@OliverO2 It is with a sad feeling that we see an important and hard working contributor leaving, but that is part of a life-time of an Open Source project. We stay positive and it is as you already mentioned - ReaR is mature enough to survive the storm...

jsmeix commented at 2021-07-02 10:31:¶

I think https://github.com/rear/rear/pull/2633
provides - at least most of - what is wanted by this pull request here
but without having possibly inexplicable stdout/stderr messages
in the log in non-debug modes.
So I think https://github.com/rear/rear/pull/2633
obsoletes this one - at least to a sufficiently large extent in practice.

[Export of Github issue for rear/rear.]

#2627 PR closed: Enable a clean log file without suppressing stderr unconditionally¶