Skip to content

Comments

fixed build.yml#417

Open
sarahd-wu wants to merge 13 commits intoharvard-lts:mainfrom
sarahd-wu:main
Open

fixed build.yml#417
sarahd-wu wants to merge 13 commits intoharvard-lts:mainfrom
sarahd-wu:main

Conversation

@sarahd-wu
Copy link

Resolves Issue: #415

What does this Pull Request do?

  • Updated exiftool download version from 13.36 to 13.49.
  • Fixed checksum values.
  • Fixed directory PATH issue. Recursively searched for .exe file instead of assuming it is in the current directory.

Note:

On my local environment, the build.yml is still not passing all of the tests.

[ERROR] Tests run: 120, Failures: 70, Errors: 2, Skipped: 21
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for FITS Parent 1.6.1-SNAPSHOT:
[INFO]
[INFO] FITS Parent ........................................ SUCCESS [ 0.157 s]
[INFO] FITS Tika .......................................... SUCCESS [ 1.895 s]
[INFO] FITS DROID ......................................... SUCCESS [ 1.689 s]
[INFO] FITS JHOVE ......................................... SUCCESS [ 0.339 s]
[INFO] FITS Tool Installer ................................ SUCCESS [ 1.494 s]
[INFO] FITS ............................................... FAILURE [01:40 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE

Copy link
Contributor

@awoods awoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Github actions now succeed!
https://github.com/harvard-lts/fits/actions/runs/21702943581/job/62624056240

I am confirming locally with:

docker build -f docker/Dockerfile-test -t fits-test .
docker run --rm -v `pwd`:/fits -v ~/.m2:/root/.m2 fits-test mvn clean test

In the meantime, please remove the two mentioned files... then we can merge after my local tests successfully complete. Thanks!

.project Outdated
<nature>org.eclipse.m2e.core.maven2Nature</nature>
<nature>org.eclipse.wst.common.project.facet.core.nature</nature>
</natures>
<filteredResources>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from the pull-request. Not everyone uses Eclipse.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the file!

@@ -0,0 +1,4 @@
activeProfiles=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from the pull-request. Not everyone uses Eclipse.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the file!

@sarahd-wu
Copy link
Author

The build broke again! :( Though it makes sense now because the GitHub Action errors match with my local environment. I believe the Eclipse-related files influenced how the metadata was ordered in some of the XML files. So the expected XML file found child node position 5 at position 9...

I believe Eclipse influences the order in how tools are added onto the PATH? so may take a look into the bug more. I wonder if the solution would either be to 1) fix/edit the output of the expected unit tests, 2) standardize the order of the tools are added to the XML metadata in the source code, 3) or if there's anything else.

@awoods
Copy link
Contributor

awoods commented Feb 6, 2026

@pwinckles : do you thoughts on this unit test question?

@pwinckles
Copy link
Contributor

@awoods @sarahd-wu No, I don't believe the failures are related to tool loading order. I can't see the CI build, so I'll just have to assume that it's the same failure that I see locally:

image

This is showing that in the expected (left) there's a conflict entry coming from NLNZ for the created timesamp, and that element isn't present in the actual xml (right). This means that the NLNZ tool is producing an output that agrees with one or the created timestamps that is present. I inspected the output, and saw that it was indeed producing 2010-03-18T17:36:00. Why this changed, I couldn't say off hand, but to fix the tests simply delete the NLNZ created elements from the expected xml files in question.

When upgrading a tool, it's very normal for tests to break. Each of the failures has to be evaluated to determine if the code that processes the tool output needs to be updated to address a change in output from the underlying tool, or if the test needs to be updated. In this case, the failure wasn't related to the tool change, but the diagnostic process is the same.

@sarahd-wu
Copy link
Author

@pwinckles @awoods Thank you for your input. Your explanation makes sense. I'll take a look at changing the test outputs for the NLNZ created elements.

I am encountering another issue with my local environment. My XML expected output has <tool toolname="MediaInfo" toolversion="22.09" status="did not run" /> , but my XML actual output has <tool toolname="MediaInfo" toolversion="[could not launch tool]" status="failed" />. I believe that the MediaInfo tool on my computer is installing incorrectly.

image

I am looking at the error logs, and there seems to be an issue opening the exe for MediaInfo, or /fits/tools/mediainfo/linux/libmediainfo.so.0: cannot open shared object file: No such file or directory. Apparently, Native library (linux-aarch64/libmediainfo.so) not found in resource path. Which file would contain the resource path for the FITS source code?—and do you have any suggestions on how best to debug it? My first thought would be to add the exe onto PATH, but I am not quite sure how to go about it.

I am also not too sure why the MediaInfo tool only breaks in my local environment, especially if Docker should create an image with the same dependencies & tools.

@pwinckles
Copy link
Contributor

@sarahd-wu Are you trying to run the tests from within your IDE? They can work, assuming you have all of the tools installed correctly locally, but the results will be inconsistent with what's produced when they're run within docker. Because the results are so system dependent (for example, things like version of the file utility, version of mediainfo, and timezone), the source of truth for the test output is when they're run within the docker container.

I don't have time at the moment to troubleshoot what's going on with your mediainfo install, but note the following from the readme:

The linux copy of MediaInfo distributed with FITS was built for Ubuntu Jammy. It may work on other distributions as well, but, if it doesn't, install MediaInfo directly on your system and delete the copy in FITS at tools/mediainfo. After deleting the copy in FITS, it will then use the system copy.

What operating system and architecture are you running?

@pwinckles
Copy link
Contributor

@sarahd-wu Again, assuming you're trying to run the tests in your IDE, are you wanting to use your system media info, or one from within the FITS project? In either case, what do you have under tools/mediainfo in the FITS project?

@sarahd-wu
Copy link
Author

@pwinckles Apologies! My wording is a little off. I meant that I am running FITS on a Docker image within my local environment. I am using the MediaInfo tool from within the FITS project, and the exe (or the libmediainfo.so.0: file) is in /tools/mediainfo/linux.

I am using macOS on an Apple M3 chip.

I noticed the README suggested to manually install MediaInfo if I am not using Ubuntu. I should be using the Ubuntu OS (FROM docker.io/eclipse-temurin:17-jre-jammy inside of Docker), but an error still throws. I think it is because MediaInfo is a Native library, so maybe my operating system architecture is still impacting the run? Idk. \o/

I think I will try # docker run --rm -it -v pwd:/fits:z -v ~/.m2:/root/.m2:z fits-test and create an interactive Docker container access, and maybe try to manually install MediaInfo from inside the Docker image.

@pwinckles
Copy link
Contributor

@sarahd-wu hmm. It should work fine within the container. That's how I run the tests and how they run in CI. Can you tell me the following:

  1. What do you see if you execute tree tools/mediainfo?
  2. What is the exact command that you're executing when you get the error?

@sarahd-wu
Copy link
Author

@pwinckles I am running docker run --rm -v pwd:/fits:z -v ~/.m2:/root/.m2:z fits-test mvn clean test

Result after tree tools/mediainfo.
image

@pwinckles
Copy link
Contributor

@sarahd-wu A couple of things.

First, this is indeed an architecture issue. The pre-build linux binaries distributed from mediainfo are all for amd64, and won't work in your aarch image. To address this, add --platform=linux/amd64 to all of the docker commands. For example:

docker build --platform=linux/amd64 -f docker/Dockerfile-test -t fits-test .

Second, in this MR, you upgraded exiftool to 13.49. However, 13.49 is not a "production" release, which means it disappears very quickly, and has already been removed from their website. When upgrading exiftool, you should first check their history page (https://exiftool.org/history.html), and only select versions that are marked as "production release".

@pwinckles
Copy link
Contributor

@sarahd-wu Another option, instead of forcing an amd64 container, is to change the test image to install mediainfo from the package repo. You'd also need to make sure that the bundled copy was not present within the FITS tools directory, and it'd likely be a different version.

Personally, I never liked that FITS bundled mediainfo because the bundled version does not work on every system.

@sarahd-wu
Copy link
Author

@pwinckles Thank you so much. The --platform flag works. I've also changed the Exiftool version, and will keep what you clarified about production releases in mind for the future.

I am looking into the NLNZ timestamp issue. I would like to look into why the NLNZ line isn't generated for the actual output, before deleting the NLNZ line from the expected test to match the actual output. I think I may try adding breakpoints to the src while running a unit test (DocMDXml) to see what happens. I've been watching a few videos on Docker & debugging with Maven's Surefire Debug plugin. Are there alternative methods to add breakpoints (or debug) the FITS tool?

Also looking into changing the current MediaInfo bundle.

@pwinckles
Copy link
Contributor

@sarahd-wu if multiple tools return the same metadata value, the value only appears once in the output associated to the highest ranked too. This is why the NZNL value is no longer appearing.

To debug in docker you'd have to expose a debug port. I usually just run the tests from my ide and debug there. Yeah, this will produce different results than docker, but its way easier.

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 this pull request may close these issues.

3 participants