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

question: ignored errors after compressions succeeds #22

Closed
pkubatrh opened this issue Sep 4, 2019 · 2 comments · Fixed by #25
Closed

question: ignored errors after compressions succeeds #22

pkubatrh opened this issue Sep 4, 2019 · 2 comments · Fixed by #25

Comments

@pkubatrh
Copy link
Contributor

pkubatrh commented Sep 4, 2019

Hi @vapier!
I recently took over maintenance for this project's rpm packages in Fedora/RHEL and wanted to follow up with you regarding a question one of our users had in regards to $SUBJECT.

If I understand the code correctly, after finishing the compression, ncompress tries to call chmod/chown on the output file (eg. here), trying to change its mode/owner to values the original file had. In case the operation does not succeed (I guess when the owner of the original file is different than the one running the compression?) the errors from the calls are handled (but ignored as the compression is already done) and the exit value of the program is set to "1".

I wonder, should the exit value of the program be still set to "1", even if the errors were "ignored" and (as far as I can understand) did not really affect the result of the compression? Or is there something I am missing here?

@vapier
Copy link
Owner

vapier commented Oct 23, 2019

the error messages & the exit behavior are def out of sync. the messages say "ignored" but then clearly aren't.

looks like the 4.2.4 release has had this bug for utime/chmod/unlink, and i copied the bug for chown for the 4.2.4.3 release.

the question is whether we should make them actually ignored. if we look at gzip for inspiration, it ignores all of these fields (after emitting the warnings):
https://git.savannah.gnu.org/cgit/gzip.git/tree/gzip.c?h=v1.10#n1970

feel like sending a PR to do that ? i.e. delete the exit_code = 1; lines in these four cases.

pkubatrh added a commit to pkubatrh/ncompress that referenced this issue Oct 23, 2019
vapier pushed a commit that referenced this issue Oct 23, 2019
@pkubatrh
Copy link
Contributor Author

feel like sending a PR to do that ? i.e. delete the exit_code = 1; lines in these four cases.

Done, thanks for taking a look!

I actually noticed one more spot where exit_code = 1; is used even though the return value should be ignored. This one does not seem like a bug as the value is overwritten directly after, but I have removed it anyway, to have all the handlers in sync.

vapier pushed a commit that referenced this issue Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants