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

Remove gtest DLLs from windows releases #1984

Closed
piponazo opened this issue Oct 22, 2021 · 6 comments
Closed

Remove gtest DLLs from windows releases #1984

piponazo opened this issue Oct 22, 2021 · 6 comments
Labels
CMake Configuration issues related with CMake release windows Windows Specific issues

Comments

@piponazo
Copy link
Collaborator

I noticed that in the last 0.27.5 windows release, the gtest DLLs are being deployed:

Screenshot from 2021-10-22 07-38-11

I think those files are only needed for testing the different executables on the CI pipelines. Probably we can skip them for the final releases.

@piponazo piponazo mentioned this issue Oct 22, 2021
@piponazo piponazo added CMake Configuration issues related with CMake release windows Windows Specific issues labels Oct 22, 2021
@clanmills
Copy link
Collaborator

I'm not going to delay releasing v0.27.5 to deal with this. These artefacts are in every release since 0.27. Before 0.27, the windows build was an MinGW build which was cross compiled on Linux. The use of gtest began with v0.27.

I think this can be fixed in cmake/packaging.cmake. I don't see any code to deliberately add gmock/gtest to the bin, so I think they were copied into the bin to run unittest.exe. So those 4 dlls ended up in the release by default and we should add code to remove them from the package.

In discussing #1982, I've realised that we are building exiv2.dll and a static exiv2.lib. I don't think this is a good idea. We should build a dll and a stub (implib) library and use dynamic run-time. I suspect that #1960 will go away when the public build provides a stub library instead of a static library.

@clanmills
Copy link
Collaborator

I've just done an MSVC build as part of discussing #1982 and spotted this in the build log:

conanfile.py imports(): Copied 4 '.dll' files: gtest.dll, gmock_main.dll, gtest_main.dll, gmock.dll

Now we know how those files arrived in the bin. The bin is a good place to put them because they are needed to run unit_tests.exe

Both unit tests and packaging.cmake were both new features of 0.27. I'm now certain that those 4 dlls ended up in the bundle because I didn't put any code into cmake/packaging.cmake to remove them. I didn't do that because packaging.cmake was developed before I had figured out how to run the unit tests for MSVC builds. For that matter, I think it was 0.27.1 or 0.27.2 before I figured out how to run the test harness on MSVC builds because of the dependency on bash. Ah, those were the days.

When Exiv2 recruits a release engineer, I will mentor him/her. This stuff is painful, interesting and sometimes fun! And it's really important to the well-being of Exiv2.

@piponazo
Copy link
Collaborator Author

piponazo commented Oct 23, 2021

Good investigation Robin. Indeed, the gtest binaries are deployed into the bin folder by conan, so that we can run the unit tests later. I see at least two options to not include those DLLs in the final artifacts:

  • In the CI Github Action we have for the releases (link) we could disable the unitTests option from the conan recipe (link). In that way conan will bring all the dependencies but the gtest one. (Preferred option)
  • Another option would be to just remove those files manually before using CPack.

@clanmills
Copy link
Collaborator

clanmills commented Oct 23, 2021

I think it's valuable to run the unit tests as part of the build.

We should remove the four DLLs from the bundle in cmake/packaging.cmake. I think the bundle is created by CPack in a temporary location and then compressed to a zip or tar.gz. So removing them from the bundle does nothing to the build tree itself.

@piponazo
Copy link
Collaborator Author

I have been investigating this topic and I think we do not have a problem there when generating the packages with the github action defined in the file .github/workflows/release.yml.

We can see the files being generated at the moment with here:
https://github.com/Exiv2/exiv2/releases/tag/untagged-e86d50a8436d2aaee906

Maybe the windows package for 0.27.5 was generated manually?
https://github.com/Exiv2/exiv2/releases/tag/v0.27.5

@piponazo
Copy link
Collaborator Author

Confirmed, I have triggered a manual execution of the release.yml pipeline on 0.27 and the windows package does not contain the gtest DLLs:

https://github.com/Exiv2/exiv2/actions/runs/1566651016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Configuration issues related with CMake release windows Windows Specific issues
Projects
None yet
Development

No branches or pull requests

2 participants