If headless conversion failed or aborted, as seen with examples from bug 97392 or bug 148253, it still returns error status 0. It should return some specific error. To reproduce, run a script with: /instdir/program/soffice --headless --convert-to xlsx password-to-open.ods echo "status is $?"
I agree that we need to introduce a "*some* commands failed" return code. Note that a command line is not necessarily a simple "soffice file", but it may have multiple files, and multiple commands (allowing one to print several files, and convert several files at the same command line). So it is possible that only *some* of them failed. Additionally, failures are different: some may be absent; some may be corrupt, or couldn't be exported using unavailable export filters ... so it is not a simple task. The "some error processing some file" is very different from other kinds of failures (that set the error codes) - like crashes, restart requests, etc. Interesting, what is a convention commonly used in such a case?
Automated bibisect cannot be done now for those cases. I think that there should be multiple status errors for different cases. Why not start for a simple and most common case of single conversion that fails or is aborted or source file not found. These are reserved Bash exit codes https://tldp.org/LDP/abs/html/exitcodes.html.
Stephan, do you know if that's feasible? IIRC, we can return rather arbitrary stuff coming from some external APIs, so can we declare some private error codes range, and make it public API?
(In reply to Mike Kaganski from comment #3) > Stephan, do you know if that's feasible? IIRC, we can return rather > arbitrary stuff coming from some external APIs, so can we declare some > private error codes range, and make it public API? I don't understand the above. Why not keep things simple and if any --convert-to operation fails call std::exit with EXIT_FAILURE rather than EXIT_SUCCESS.
(In reply to Stephan Bergmann from comment #4) > Why not keep things simple and if any --convert-to operation fails call > std::exit with EXIT_FAILURE rather than EXIT_SUCCESS. If that's an option - then great! Do you mean "std::exit after the first failure"? How a user would know if some previous ones succeeded? I suppose that EXIT_FAILURE is more about inability to run. But having another dedicated value for the case would likely make it simple enough, and still distinguishable?
(In reply to Mike Kaganski from comment #5) > I suppose that EXIT_FAILURE is more about inability to run. ... as *we* use it, not implying it's what stdlib means :-)
(In reply to Mike Kaganski from comment #5) > Do you mean "std::exit after the first failure"? What I had thought about was to change the code so that where apparently it currently unconditionally does the equivalent of std::exit(EXIT_SUCCESS), make it do the equivalent of std::exit(EXIT_FAILURE) if any of those --convert-to operations had failed (after processing all of them). But stopping after the first failing one might be fine as well. > How a user would know if some previous ones succeeded? Why would we care? If the user wants to have more precise information about which operations succeeded or failed, they could either call soffice for each of them individually, or they could presumably use the UNO API to do the same operations with more fine grained error reporting. > I suppose that EXIT_FAILURE is more about inability to run. But having > another dedicated value for the case would likely make it simple enough, and > still distinguishable? EXIT_FAILURE is all that the C/C++ standards portably offer. It traditionally translates to an exit status of 1 on Posix (and Windows), which traditionally represents a catch-all "failure" status in those environments. I still fail to see a need for a more specific exit status for "some --convert-to operation failed" in those environments.
I think it's safe to put New. Anything is better than current 0 so a single error would be good and this bug could be closed. As for why more would be better, I listed some cases: conversion fails or is aborted or source file not found or source password protected.
I noticed another aspect of this. soffice-convert && app-to-open doesn't work, at least in Windows, because soffice immediately gives 0 for %errorlevel% so app-to-open opens immediately after soffice-convert is run, when no converted file exists.
(In reply to Timur from comment #9) What is "soffice-convert"? Is the proper console mode `soffice.com` used, that is intended to work properly in command line and batch mode [1], and allows use of the program name without extension from command line? [1] https://mikekaganski.wordpress.com/2018/11/21/proper-console-mode-for-libreoffice-on-windows/
(In reply to Mike Kaganski from comment #10) > (In reply to Timur from comment #9) > What is "soffice-convert" That was meant to be: soffice.exe --headless --convert-to ext > Is the proper console mode `soffice.com` used, > that is intended to work properly in command line and batch mode [1], and > allows use of the program name without extension from command line? No, I just learned about it. And really it's working properly, it waits for a command to finish to continue with the next one. Thanks.
Dear Timur, To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/ If the bug is present, please leave a comment that includes the information from Help - About LibreOffice. If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice. Please DO NOT Update the version field Reply via email (please reply directly on the bug tracker) Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to 'inherited from OOo'; 4b. If the bug was not present in 3.3 - add 'regression' to keyword Feel free to come ask questions or to say hello in our QA chat: https://web.libera.chat/?settings=#libreoffice-qa Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team MassPing-UntouchedBug
(In reply to Stephan Bergmann from comment #7) > > How a user would know if some previous ones succeeded? > Why would we care? +1 > EXIT_FAILURE is all that the C/C++ standards portably offer. So IMHO that makes it a simple decision - the choice is already made for us.
So a code pointer: RequestHandler::ExecuteCmdLineRequests processes aRequest.aConversionParams. It happens in the main instance of the program. We need to keep track of each of the processed conversion results. When it's initiated by its own main() (through Desktop::OpenClients), it needs to make sure, that when main() ends, it returns the appropriate code. When it's called from IpcThread::process, the end result must be transferred back to the calling instances.
proposed patch at https://gerrit.libreoffice.org/c/core/+/187075
A code pointer for transferring status to the external caller: desktop/unx/source/start.c : send_args (Linux?) / desktop/source/app/officeipcthread.cxx : PipeIpcThread::enable (general / windows) sends args and receives the status; it checks if it got "InternalIPC::ProcessingDone". We could return other strings instead (just need to fit into the length?). Returning the status happens in desktop/source/app/officeipcthread.cxx : PipeIpcThread::execute; and there we can check the new "GetAllSucceeded".
Justin Luth committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/633ce6b17007ea401aa5c013ec14308b14596d31 tdf#148275: EXIT_FAILURE if any cmdline operation fails It will be available in 26.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Mike Kaganski from comment #16) > A code pointer for transferring status to the external caller: I'm not sure under what steps-to-reproduce this would be processed, so I haven't done anything more than a code-read. Assuming this can be made into a separate bug report if desired. > Returning the status happens in desktop/source/app/officeipcthread.cxx : > PipeIpcThread::execute; and there we can check the new ... m_handler->mFlags & DispatchRequestFlags::WithError); I hope that my "design" allows this to be nicely extended in case more granularity or other kinds of "result flags" are desired in the future. Marking this bug as fixed.
This is IMO worth ReelaseNotes.