github CI: use ilammy/msvc-dev-cmd

[neheb]

Small simplification

[ghost]

👇 Click on the image for a new way to code review

Legend

[codecov]

Codecov Report

Merging #2514 (eecd473) into main (4a6d786) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head eecd473 differs from pull request most recent head 9626c62. Consider uploading reports for the commit 9626c62 to get more accurate results

@@            Coverage Diff             @@
##             main    #2514      +/-   ##
==========================================
+ Coverage   64.65%   64.66%   +0.01%     
==========================================
  Files         103      103              
  Lines       22232    22249      +17     
  Branches    10858    10859       +1     
==========================================
+ Hits        14373    14388      +15     
- Misses       5618     5620       +2     
  Partials     2241     2241              

Impacted Files	Coverage Δ
include/exiv2/matroskavideo.hpp	56.25% <0.00%> (-3.75%)	⬇️
src/value.cpp	73.40% <0.00%> (-0.45%)	⬇️
src/tiffimage.cpp	71.94% <0.00%> (-0.40%)	⬇️
src/minoltamn_int.cpp	69.46% <0.00%> (-0.27%)	⬇️
src/http.cpp	0.00% <0.00%> (ø)
app/actions.cpp	67.91% <0.00%> (ø)
src/basicio.cpp	51.90% <0.00%> (ø)
src/sonymn_int.cpp	80.48% <0.00%> (ø)
src/matroskavideo.cpp	4.52% <0.00%> (ø)
include/exiv2/metadatum.hpp	78.57% <0.00%> (ø)
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[neheb]

Seems x86 builds are bugged:

======================================================================
ERROR: issue_1912_poc.jpg_test (test_regression_allfiles.TestAllFiles.issue_1912_poc.jpg_test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\exiv2\exiv2\tests\regression_tests\test_regression_allfiles.py", line 191, in test_func
    BT.reportTest(os.path.basename(filename), out)
  File "D:\a\exiv2\exiv2\tests\bash_tests\utils.py", line 567, in reportTest
    raise RuntimeError('\n' + log.to_str())
RuntimeError: 
Error:  The output of the testcase mismatch the reference
[INFO] The output has been saved to file D:\a\exiv2\exiv2\test\tmp\issue_1912_poc.jpg.out
[INFO] simply_diff:
D:\a\exiv2\exiv2\test/data\test_reference_files\issue_1912_poc.jpg.out: 53 lines
D:\a\exiv2\exiv2\test\tmp\issue_1912_poc.jpg.out: 53 lines
The first mismatch is in line 49:
< Exif.GPSInfo.GPSTimeStamp                    Rational    3  1131375984/1869505902 1950679091/1 18/1  05:20:00
> Exif.GPSInfo.GPSTimeStamp                    Rational    3  1131375984/1869505902 1950679091/1 18/1  05:20:18

[neheb]

@kevinbackhouse so the value on the bottom is

07:36:50

on linux. Sounds like UB.

[kevinbackhouse]

@kevinbackhouse so the value on the bottom is

07:36:50

on linux. Sounds like UB.

@neheb: Is UB an abbreviation for undefined behavior? Is this test failing sporadically?

[neheb]

Yes. Not sporadic at all.

[neheb]

@kevinbackhouse added back 32-bit tests.

[neheb]

https://gist.github.com/neheb/d19b90a1a21214aa4e48ae7832bfcfe9

The failures are the same on x86 linux

[kevinbackhouse]

Speculating a bit here, because I'm debugging on 64-bit Linux, but most likely it's caused by a difference in floating point accuracy in this calculation:

exiv2/src/tags_int.cpp

Line 2615 in 79bf4d0

const double t = 3600 * value.toFloat(0) + 60 * value.toFloat(1) + value.toFloat(2);

[neheb]

partially.

So I tried fixing this by converting a bunch of doubles to floats. On 32-bit linux closest I got was 5:20:00.0

[kevinbackhouse]

partially.

So I tried fixing this by converting a bunch of doubles to floats. On 32-bit linux closest I got was 5:20:00.0

I was wondering if the opposite would help: use double, not float. Experiment is here: #2520

[neheb]

Might want to add the second commit in this PR to that one.

[neheb]

remaining failures are CVE failures. Can you take a look?

[kevinbackhouse]

I looked at test_CVE_2017_14864 first. On x86 it's throwing an exception in a Safe::add<size_t>() which obviously overflows more easily on 32-bit than on 64-bit. The result is just that it prints a different error message than the test is expecting, so that should be easy to fix in the test.

[kevinbackhouse]

#2522 will hopefully fix the remaining problems

[neheb]

Thanks to @kevinbackhouse , this passes now.