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 unnecessary headers from strutil.cpp causing build trouble #3976

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

jessey-git
Copy link
Contributor

Description

While testing out the 2.5 beta for use in Blender, I hit an odd build error[1] apparently related to include order. Apparently windows.h should be included before shellapi.h[2]. Swapping the order does fix things, however, it turns out neither is used in this particular file so just remove them both. Unsure why we didn't see it until now.

[1]

20>[9/146] Building CXX object src\libutil\CMakeFiles\OpenImageIO_Util.dir\strutil.cpp.obj
20>FAILED: src/libutil/CMakeFiles/OpenImageIO_Util.dir/strutil.cpp.obj

20>cl : Command line warning D9002: ignoring unknown option '/arch:sse2'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): error C2146: syntax error: missing ';' before identifier 'DECLSPEC_IMPORT'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(79): error C2065: 'HDROP': undeclared identifier
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(82): error C2086: 'int EXTERN_C': redefinition
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): note: see declaration of 'EXTERN_C'

[2]
https://stackoverflow.com/questions/2953704/vs2010-lots-of-errors-when-including-standard-libraries
https://social.msdn.microsoft.com/forums/vstudio/en-us/ed93ae48-969a-4280-8fd5-ef57a332022b/problems-with-shellexecute-and-shellapih?forum=vcgeneral

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • If I added or modified a C++ API call, I have also amended the
    corresponding Python bindings (and if altering ImageBufAlgo functions, also
    exposed the new functionality as oiiotool options).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

Signed-off-by: Jesse Yurkovich <[email protected]>
@lgritz
Copy link
Collaborator

lgritz commented Sep 8, 2023

Why didn't this fail on our Windows CI?

@jessey-git
Copy link
Contributor Author

I'm really not sure. windows.h was added to this file as part of 83c8853 but as to why CI didn't catch it, I'm guessing some difference in environment as I'm using vs2022 Preview while CI is still on the vs2019 toolsets.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@lgritz lgritz merged commit 0d9b971 into AcademySoftwareFoundation:master Sep 8, 2023
23 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Sep 8, 2023
…ademySoftwareFoundation#3976)

While testing out the 2.5 beta for use in Blender, I hit an odd build
error[1] apparently related to include order. Apparently `windows.h`
should be included before `shellapi.h`[2]. Swapping the order does fix
things, however, it turns out neither is used in this particular file so
just remove them both. Unsure why we didn't see it until now.

[1]
```
20>[9/146] Building CXX object src\libutil\CMakeFiles\OpenImageIO_Util.dir\strutil.cpp.obj
20>FAILED: src/libutil/CMakeFiles/OpenImageIO_Util.dir/strutil.cpp.obj

20>cl : Command line warning D9002: ignoring unknown option '/arch:sse2'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): error C2146: syntax error: missing ';' before identifier 'DECLSPEC_IMPORT'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(79): error C2065: 'HDROP': undeclared identifier
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(82): error C2086: 'int EXTERN_C': redefinition
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): note: see declaration of 'EXTERN_C'
```
[2] 

https://stackoverflow.com/questions/2953704/vs2010-lots-of-errors-when-including-standard-libraries

https://social.msdn.microsoft.com/forums/vstudio/en-us/ed93ae48-969a-4280-8fd5-ef57a332022b/problems-with-shellexecute-and-shellapih?forum=vcgeneral

## Checklist:

<!-- Put an 'x' in the boxes as you complete the checklist items -->

- [X] I have read the [contribution
guidelines](https://github.com/OpenImageIO/oiio/blob/master/CONTRIBUTING.md).
- [ ] I have updated the documentation, if applicable.
- [ ] I have ensured that the change is tested somewhere in the
testsuite
  (adding new test cases if necessary).
- [ ] If I added or modified a C++ API call, I have also amended the
corresponding Python bindings (and if altering ImageBufAlgo functions,
also
  exposed the new functionality as oiiotool options).
- [X] My code follows the prevailing code style of this project. If I
haven't
already run clang-format before submitting, I definitely will look at
the CI
test that runs clang-format and fix anything that it highlights as being
  nonconforming.

Signed-off-by: Jesse Yurkovich <[email protected]>
@jessey-git jessey-git deleted the fixwin branch September 26, 2023 02:48
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.

2 participants