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

Consider removing or renaming VERSION file #191

Closed
MoAlyousef opened this issue Feb 18, 2021 · 6 comments
Closed

Consider removing or renaming VERSION file #191

MoAlyousef opened this issue Feb 18, 2021 · 6 comments
Assignees
Labels
fixed The issue or PR was fixed. Platform: Windows platform specific (Windows)

Comments

@MoAlyousef
Copy link
Contributor

MoAlyousef commented Feb 18, 2021

Hello

Android and Emscripten’s clang toolchain on Windows mistake fltk’s VERSION file for libcxx c++20 “version” header and would try to include it internally (because of Window’s non case-sensitive filesystem.
I haven’t tried to use clang that is bundled with msvc but it could potentially have the same issue.

@Albrecht-S Albrecht-S self-assigned this Feb 19, 2021
@Albrecht-S Albrecht-S added active Somebody is working on it Platform: Windows platform specific (Windows) question Further information is requested labels Feb 19, 2021
@Albrecht-S
Copy link
Member

Thanks for the report. I can probably rename this file, it's just a build and maintenance support file.

I assume:

  • it is found when you build your application (not the FLTK lib itself) and
  • it is found only if you build your application directly against the FKTK build directory
  • it is not found if you build your app after installing FLTK into an arbitrary (not necessarily system default) location and building against this directory.

Can you confirm all or any of these assumptions? Which ones?

Other questions for my better understanding of the issue:

  • what build system are you using (CMake, configure/make, ...)?
  • if CMake: directly in the source tree (not recommended!) or out of source, i.e. in a separate build directory?

@fltk fltk deleted a comment Feb 19, 2021
@MoAlyousef
Copy link
Contributor Author

MoAlyousef commented Feb 19, 2021

It's when using CMake, in both cases invoking a CMake toolchain file, and having FLTK built with the application/library I'm building. Also just trying to build FLTK as a library before installing it. So options 1, 2 and 3 are true.
Example usage:

$ git clone --depth 1 https://github.com/fltk/fltk
$ cd fltk
$ cmake -Bbin -S. -DFLTK_BUILD_TEST=OFF -DCMAKE_TOOLCHAIN_FILE=D:\Android\Sdk\ndk\21.1.6352462\build\cmake\android.toolchain.cmake
$ cmake —build bin —parallel

Would fail.

Also using Android Studio:

  • Create a native C++ app, accept defaults.
  • Navigate to the Android generated CMakeLists
  • Git clone or git submodule add fltk in the same directory.
  • Replace contents of CMakeLists with:
cmake_minimum_required(VERSION 3.10)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -u ANativeActivity_onCreate")
set(FLTK_BUILD_TEST OFF CACHE BOOL " " FORCE)
add_subdirectory(fltk)
add_library(native-lib SHARED native-lib.cpp)
target_include_directories(native-lib PRIVATE fltk ${CMAKE_BINARY_DIR}/fltk)
target_link_libraries(native-lib PRIVATE fltk fltk_images fltk_jpeg fltk_z fltk_png)
target_link_libraries(native-lib PUBLIC log android)
  • Replace native-lib.cpp code with any FLTK code.
  • Run the build (invokes gradle which calls cmake).

@Albrecht-S
Copy link
Member

Okay, thanks for the quick confirmation and the build details.
I'll rename the offending file.

@MoAlyousef
Copy link
Contributor Author

Thank you. Simply changing the extension works as well.

@Albrecht-S Albrecht-S added fixed The issue or PR was fixed. and removed active Somebody is working on it question Further information is requested labels Feb 20, 2021
Albrecht-S pushed a commit that referenced this issue Feb 20, 2021
The file VERSION could be included erroneously on case insensitive
platforms (notably Windows) if the user included the c++ standard
header <version> directly or indirectly.

Renaming the file fixes this.
@Albrecht-S
Copy link
Member

I did both: rename to another file name and extension.

@MoAlyousef
Copy link
Contributor Author

Thank you

Albrecht-S pushed a commit that referenced this issue Apr 27, 2021
The file VERSION could be included erroneously on case insensitive
platforms (notably Windows) if the user included the c++ standard
header <version> directly or indirectly.

Renaming the file fixes this.

Backported from 1.4 (master).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed The issue or PR was fixed. Platform: Windows platform specific (Windows)
Projects
None yet
Development

No branches or pull requests

2 participants