Feature #917

Modify exiv2/actions.cpp return -3/253 when no metadata has been found.

Added by Robin Mills almost 4 years ago. Updated 10 months ago.

Status:ClosedStart date:11 Aug 2013
Priority:NormalDue date:
Assignee:Andreas Huggel% Done:

100%

Category:designEstimated time:2.00 hours
Target version:0.26

Description

See discussion:
http://dev.exiv2.org/boards/3/topics/1582

Additionally:
1) we should print the error message when we set the error. We should not require -v (verbose) to be set to get this error message.
2) when -g (grep) has been used, we should not set an error if no tag matches

0917_supportregexp.patch Magnifier (3.76 KB) Thomas Schmidt, 09 Jan 2015 22:49

0917_returnvaluesanderror.patch Magnifier (2.24 KB) Thomas Schmidt, 13 Jan 2015 23:05


Related issues

Related to Exiv2 - Feature #1024: Provide regular expression support for the exiv2 -g feature Closed 10 Jan 2015
Related to Exiv2 - Bug #918: non-zero exit code when extracting thumbnails Closed 25 Aug 2013

Associated revisions

Revision 3566
Added by Robin Mills over 2 years ago

#917. Thank you Thomas for the patch. Very much appreciated.

Revision 3567
Added by Robin Mills over 2 years ago

#917. Changing the condition for reporting an error. We should not report an error for missing metadata when the user uses [-g match]+

Revision 3901
Added by Andreas Huggel almost 2 years ago

#917: Tweaks to the utility -p<x> return codes and info messages.

Revision 3904
Added by Andreas Huggel almost 2 years ago

#917: Updated test results.

History

#1 Updated by Andreas Huggel almost 4 years ago

Both current behaviors 1) and 2) are designed to meet the needs of both, users of the command line program - they can set -v to see some extra messages - and scripts - it is more concise to check the return value in a script than to parse the program's standard error output and it is easier if the program doesn't generate a lot of unneeded output. In addition, the behavior of the -g (grep) option follows that of the grep command, which also returns a non-zero value if it can't find what you're looking for. I guess one could argue what the default behavior should be (i.e., whether the program should output the -v messages by default and have a switch to suppress them instead), but I wouldn't be surprised if you can find an equal number of supporters for each of these options...

#2 Updated by Robin Mills almost 4 years ago

Thanks for adding your comments, Andreas.

Are you saying that you think we should not do anything about this?

It's possible to make a case that the existing behavior is correct and requires no change. Having discussed the matter in Topic 1582, Torsten has stated that he can work-around the current behavior.

If you consider that we should do nothing, then please close this issue/feature as "Rejected".

#3 Updated by Robin Mills over 2 years ago

  • Assignee changed from Robin Mills to Thomas Schmidt

Thomas

May I ask you to have a look at this please? This is a "gray one" in which the existing behaviour is neither right nor wrong. I don't remember the details here, however you can see my thoughts when I opened this issue in response to topic 1582.

If you would like a bigger challenge, it would be good to make the -g option respect regular expressions. However do not use boost. The build consequences of boost outweigh the benefits of -g regular-expression. If you know/find decent public domain code for regular-expressions, can you add it to utils.cpp and use it to support the -g option?

Robin

#4 Updated by Thomas Schmidt over 2 years ago

On Linux, the GNU regex.h library seems to be available. I created a patch that will add support for regular-expressions in -g (attached), but I'm not sure if that compiles on all platforms...

#5 Updated by Thomas Schmidt over 2 years ago

Patch attached for the following behavior:

o Return code is only an error (-3 i.e. 253) if none of the requested meta data type is present: when all types are requested (-pa), any type suffices. When a specific type (e.g. IPTC, -pi) is requested, some data of its type must be present.

o If tag filters are given, no error is returned when no key matches.

o Error "No (Exif|IPTC|XMP) data found in the file" is reported without giving -v argument, but only if no data of another requested type is present.

#6 Updated by Robin Mills over 2 years ago

r3566 Thank you very much to Thomas for working on this and providing the patch.

I've modified Thomas code slightly and hope not to have introduced a bug. If I have, I'll take ownership and fix it. I've also updated test files which are impacted. The test file output is modified because it now (correctly in my opinion) reports when no metadata is available to match the request.

Thomas: you have been extremely help on #917 and #1024. Thank You very much for getting involved with Exiv2.

#7 Updated by Robin Mills over 2 years ago

  • Status changed from Assigned to Resolved
  • Assignee changed from Robin Mills to Thomas Schmidt

I'm going to mark this as 'Resolved" as it builds and passes the test suite on the build server. http://exiv2.dyndns.org:8080/job/Exiv2-trunk/1835/

I've also changed the Assignee to Thomas Schmidt to ensure he is credited with this fix when the v0.25 release notes are compiled. Thank You, Thomas for working on this.

#8 Updated by Robin Mills over 2 years ago

Fix submitted: r3566 and r3567.

I've added this to try to persuade Redmine to associate those submissions with this issue.

#9 Updated by Robin Mills over 2 years ago

  • Status changed from Resolved to Assigned

I'm going to reopen this because I'm rather unhappy with a consequence of the fix. The most common report I request with exiv2 is -pa = print all. However, if there is no IPTC metadata, a message is written to stderr, however the return code is set to 0.

581 rmills@rmillsmbp:~/gnu/exiv2/trunk/test $ exiv2 -pa ~/R.jpg | wc
/Users/rmills/R.jpg: (No IPTC data found in the file)
     177     837   12153
582 rmills@rmillsmbp:~/gnu/exiv2/trunk/test $ echo $?
0
583 rmills@rmillsmbp:~/gnu/exiv2/trunk/test $ 
This is rather inconsistent. If we write to stderr, we should set a return code that is != 0. And equally, if we set a return code != 0, we should always write to stderr.

I am very happy with the return code of 0. I think the semantics of -pa are "give me all you can find" and no message "(No IPTC data found in the file)" should be reported by default. The options -pa --verbose should report "No IPTC data found in the file" and set a return code of -3/253. I'd also like to see us return -3 (and not 253). The man page src/exiv2.1 should be updated with a section "EXIT STATUS" and the return codes documented as recommended here: https://wiki.archlinux.org/index.php/Man_page

There are other related matters here. The man page documents -pa as being the same as -Pkyct and -pt as -PEkyct. There's something inconsistent about this (and may simply be a man page error). We should also add the option -pe to report only Exif data (to correspond to options -pi and -px which report IPTC and XMP respectively.

When we do this, we're going to change return codes and error messages. We'll need to update the reference files in the test file. This usually only involves copying output files from test/tmp/ to test/data. For example, if bugfixes.sh reports something you understand, you can silence him with:

cp tmp/bugfixes-test.out-stripped data/bugfixes-test.out
Here's a summary:

  1. We should set a return code != 0 if we write to stderr
  2. We should always return a code != 0 if we write to stderr
  3. Fix odd codes such as 253 to be -3
  4. Add option -pe to only report Exif metadata
  5. Change semantics of -pa to be silent when the file does not contain any metadata sections (Exif, IPTC, XMP)
  6. Change semantics of -pa --verbose to complain when a metadata section is missing (and set return != 0)
  7. Add a section EXIT STATUS to the man page
  8. Review if the man page -pa == -Pkyct is true
  9. Update the man page about -pe (both Command Summary and Options)
  10. Clean up the reference files in the test suite

I'll leave this assigned to Thomas S. Thomas, if you're too busy to work on this, please assign it to me and I'll take care of it.

#10 Updated by Robin Mills about 2 years ago

  • Assignee changed from Thomas Schmidt to Robin Mills

#11 Updated by Robin Mills about 2 years ago

  • Target version changed from 0.25 to 0.26

This behaviour has been in exiv2 for years. There is insufficient time in v0.25 to undertake this work. Punted to v0.26.

#12 Updated by Robin Mills about 2 years ago

  • Assignee deleted (Robin Mills)

#13 Updated by Andreas Huggel almost 2 years ago

  • Assignee set to Andreas Huggel
  • % Done changed from 0 to 50

1. We should set a return code != 0 if we write to stderr
2. We should always return a code != 0 if we write to stderr

That's difficult to implement and not practical imho. (The library may write to stderr without the utility knowing.)

3. Fix odd codes such as 253 to be -3

I don't know how to do that. It may be better to return small positive values instead.

4. Add option -pe to only report Exif metadata

There are several options to only report Exif metadata: -pt, -pv and -ph. Maybe we should rename -pt to -pe.

5. Change semantics of -pa to be silent when the file does not contain any metadata sections (Exif, IPTC, XMP)
6. Change semantics of -pa --verbose to complain when a metadata section is missing (and set return != 0)

Changed back to original: silent without -v, inform about absence of metadata with -v. In accordance with the "give me all you can find" comment I also see this as just an info message, not a complaint (warning or error), thus it now returns 0 in either case.

7. Add a section EXIT STATUS to the man page
8. Review if the man page -pa == -Pkyct is true

Yes, it is.

9. Update the man page about -pe (both Command Summary and Options)
10. Clean up the reference files in the test suite

Further, with -g or -K, exiv2 now returns 1 if no matching tags were found. That's in-line with UNIX grep.

#14 Updated by Nigel Winterbottom over 1 year ago

This bug just bit me because I'm chaining actions in a find command. Of course the non-zero exit code of exiv2 prevents later actions happening. MY usage case has not been discussed above and can be summarised thus:

$ exiv2 -V
exiv2 0.22 001600 (32 bit build)
Copyright (C) 2004-2011 Andreas Huggel.
[snipped]

nigel@D8300:~$ exiv2 -v -Pv -g Exif.Image.Model ~/Pictures/CIMG4496.JPG 
File 1/1: /home/nigel/Pictures/CIMG4496.JPG
EX-Z40 
/home/nigel/Pictures/CIMG4496.JPG: No IPTC data found in the file
/home/nigel/Pictures/CIMG4496.JPG: No XMP data found in the file
nigel@D8300:~$ echo $?
253
nigel@D8300:~$

Note I'm making no specific request to examine IPTC or XMP data.

I'm on Ubuntu 12.04LTS and was quite prepared to build the latest from source but see 0.25 does not include any fix. Please include my usage in any further test cases.

I'm going to try the perl module exiftool now - I hope it supports file renames because I have clashing file names across different Casio models and ultimately that's what I need to do.

#15 Updated by Robin Mills over 1 year ago

Thank you for your feedback on this matter. I see you are using v0.22 which is rather elderly version of exiv2. Perhaps you could try/test our daily build to see if you have a use case that defeats our fix. You can collect it from: http://exiv2.dyndns.org:8080/userContent/builds/Categorized/Platform/linux/

If you do find a flaw in our fix, I'll be happy to both repair it and and add a regression detector to our test suite.

Thank You for using exiv2.

#16 Updated by Robin Mills over 1 year ago

Your use-case appears to be fixed on the trunk:

1071 rmills@rmillsmbp:~/clanmills $ exiv2 -v -Pv -g Exif.Image.Model http://clanmills.com/Stonehenge.jpg
File 1/1: http://clanmills.com/Stonehenge.jpg
NIKON D5300
1072 rmills@rmillsmbp:~/clanmills $ echo $?
0
1073 rmills@rmillsmbp:~/clanmills $ cd ~/temp ; curl -O http://clanmills.com/Stonehenge.jpg
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 6599k  100 6599k    0     0  1658k      0  0:00:03  0:00:03 --:--:-- 1934k
1074 rmills@rmillsmbp:~/temp $ exiv2 -di Stonehenge.jpg 
1075 rmills@rmillsmbp:~/temp $ exiv2 -dx Stonehenge.jpg 
1076 rmills@rmillsmbp:~/temp $ exiv2 -v -Pv -g Exif.Image.Model Stonehenge.jpg
File 1/1: Stonehenge.jpg
NIKON D5300
Stonehenge.jpg: No IPTC data found in the file
Stonehenge.jpg: No XMP data found in the file
1077 rmills@rmillsmbp:~/temp $ echo $?
0
1078 rmills@rmillsmbp:~/temp $ 

#17 Updated by Robin Mills 10 months ago

  • Status changed from Assigned to Closed
  • % Done changed from 50 to 100

I believe this is closed.

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux