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

BinaryData.h might be included from two different folders, causing errors #573

Open
danra opened this issue Dec 29, 2019 · 4 comments
Open

Comments

@danra
Copy link

danra commented Dec 29, 2019

I had a JuceLibraryCode folder with BinaryData.h under my root project folder from a previous non-FRUT build, and after building with FRUT also under build/JuceLibraryCode.

The two files both got included in some source files, causing variable redefinition errors.

Perhaps add a step to the README suggesting to remove JuceLibraryCode from non-FRUT builds to make sure this doesn't happen?

Or, try to handle it via include paths, but I guess this could prove tricky for all projects (e.g. in my project I had ../../ as an include path, which is what Xcode needs to find some include folders under the project's root folder (without explicitly specifying ../../ every time in the include).

@danra
Copy link
Author

danra commented Jan 13, 2020

I initially thought this was an issue with my own project, but apparently JUCE's example projects also demonstrate this behavior. All that's needed to reproduce the duplicate definitions error is to first build with the Xcode project exported by the Projucer as usual, then try building with cmake (via FRUT).

@McMartin
Copy link
Owner

All that's needed to reproduce the duplicate definitions error is to first build with the Xcode project exported by the Projucer as usual, then try building with cmake (via FRUT).

I don't understand how building with the Xcode project exported by Projucer would have any impact on what the CMake-based build system does. Are you using the Builds/MacOSX folder as the binary directory for the CMake build?

@McMartin
Copy link
Owner

McMartin commented Jan 13, 2020

I can reproduce the issue when building the DemoRunner example from JUCE 5.4.5 in FRUT (https://github.com/McMartin/FRUT/tree/master/generated/JUCE-5.4.5/examples/DemoRunner):

[ 22%] Building CXX object CMakeFiles/DemoRunner.dir/Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/Main.cpp.o
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/Main.cpp:28:
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../../Assets/DemoUtilities.h:22:
/Users/alain/dev/FRUT/generated/JUCE-5.4.5/examples/DemoRunner/build/JuceLibraryCode/../JuceLibraryCode/JuceHeader.h:48:24: error: redefinition of 'projectName'
    const char* const  projectName    = "DemoRunner";
                       ^
/Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../JuceLibraryCode/JuceHeader.h:57:24: note: previous definition is here
    const char* const  projectName    = "DemoRunner";
                       ^
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/Main.cpp:28:
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../../Assets/DemoUtilities.h:22:
/Users/alain/dev/FRUT/generated/JUCE-5.4.5/examples/DemoRunner/build/JuceLibraryCode/../JuceLibraryCode/JuceHeader.h:49:24: error: redefinition of 'versionString'
    const char* const  versionString  = "5.4.5";
                       ^
/Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../JuceLibraryCode/JuceHeader.h:59:24: note: previous definition is here
    const char* const  versionString  = "5.4.5";
                       ^
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/Main.cpp:28:
In file included from /Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../../Assets/DemoUtilities.h:22:
/Users/alain/dev/FRUT/generated/JUCE-5.4.5/examples/DemoRunner/build/JuceLibraryCode/../JuceLibraryCode/JuceHeader.h:50:24: error: redefinition of 'versionNumber'
    const int          versionNumber  = 0x50405;
                       ^
/Users/alain/dev/FRUT/ci/tmp/JUCE-5.4.5/examples/DemoRunner/Source/../JuceLibraryCode/JuceHeader.h:60:24: note: previous definition is here
    const int          versionNumber  = 0x50405;
                       ^
3 errors generated.

What is happening in details:

  • DemoRunner/Source/Main.cpp:27 does #include "../JuceLibraryCode/JuceHeader.h". DemoRunner/JuceLibraryCode/JuceHeader.h is found by the compiler, relatively to Main.cpp itself, so the compiler includes it without considering the include search path.
  • DemoRunner/Source/Main.cpp:28 does #include "../../Assets/DemoUtilities.h". In the same way, the DemoUtilities.h header is found relatively to Main.cpp and included without using the include search path.
  • Assets/DemoUtilities.h:22 does #include "../JuceLibraryCode/JuceHeader.h". However, the compiler can't find a JuceHeader.h file at that relative path when starting from DemoUtilities.h, so the compiler uses the include search path. /Users/alain/dev/FRUT/generated/JUCE-5.4.5/examples/DemoRunner/build/JuceLibraryCode is in the include search path, and the compiler finds DemoRunner/build/JuceLibraryCode/JuceHeader.h as a viable candidate.

Projucer actually adds the JuceLibraryCode directory to the include search path in all exporters (Android, LinuxMakefile, MacOSX, VisualStudio2015, VisualStudio2017, VisualStudio2019, iOS), so there is no need to write #include "../JuceLibraryCode/JuceHeader.h" in any JUCE project. Replacing it with #include "JuceHeader.h" or #include <JuceHeader.h> will solve this problem.

I guess I should make a PR on JUCE to replace all existing #include "../JuceLibraryCode/*" statements and fix how Projucer generate stubs, but that won't solve the problem for existing projects... We might need a warning somewhere in FRUT as @danra suggested.

@McMartin
Copy link
Owner

McMartin commented Jan 18, 2020

I opened a pull request on JUCE to propose to always include JuceHeader.h using #include <JuceHeader.h>: juce-framework/JUCE#658.

If these changes get merged into JUCE, I will make a pull request on FRUT to:

  • change #include "JuceHeader.h" to #include <JuceHeader.h> in Jucer2Reprojucer
  • make Jucer2Reprojucer scan all source files listed in the input .jucer file for #include "../JuceLibraryCode/JuceHeader.h" and output warnings to advise changing to #include <JuceHeader.h>.

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

No branches or pull requests

2 participants