Open levicki opened 4 years ago
Hi! Sorry for the late reply and thank you very much for your feedback, I really appreciate it and I really need more issue posts like this!
I hope you are safe.
Thanks, hope you too!
I wanted to include bit7z in one of my projects as a git submodule.
This is the main reason you find most of the things you have listed as weird: bit7z was not thought to be used as a git submodule! When I first created it, I wanted the users to be able to separately compile their .lib files and use them.
What feels weird to me (kind of a brain dump after spending some time integrating it with my stuff):
- You have included
7zSDK
in thelib
subfolder of the project when those are actually source files, not prebuilt static library (or output static library) files.
Yes, the problem here is that there is no single standard project structure/layout for C++ programs/libraries. You can find lots of proposals on internet, so when I started bit7z, I mixed more than one that I found, at that time, reasonable. The result was this:
+- bit7z
+- bin // for binary outputs (including .lib files, since static libraries are (partial) binary executables)
+- include // for headers
+- lib // for 3rd party libraries ("libs" would have been a better naming, so I recently renamed it)
+- src // for sources
Now, since the next version of bit7z is going to also support Linux, I already renamed the folder to "libs" (another common name in C++ project layouts) since it will also contain the p7zip source code.
Besides, I recently managed to remove all the source code dependencies from 7-zip, and bit7z now needs only some headers files from it1.
As a side note, the name 7zSDK
is just a bad naming choice of mine since there is actually no "7z SDK" but the 7-zip source code. This often confuses people, since on the contrary there is the "LZMA SDK", which is another thing. I already addressed this issue in commit 5317efdf72c5bb05283319e03e7f25dff8fe5624.
1 Actually, this is not true for Linux, where I still need to use a single source file from p7zip.
- Your static library output is in the
bin
folder which is usually reserved for input or output executables for projects that have both. You might want to make bit7z DLL in the future, and then where will yourbit7z.dll
and corresponding import libraries go if you are putting static libraries inbin
folder?
As already mentioned, I decided to put the library output files in bin
since they are in fact binary files.
But I can understand why it can be confusing, so I am evaluating whether to change this (see at the end).
And, on a related note, the CI pre-built packages available on the GitHub releases have the binaries put in a lib
folder.
- Your
OutDir
andIntDir
directives inbit7z.vcxproj
file contain fully relative paths instead of specifying$(ProjectDir)bin\
or$(ProjectDir)build\
.
Yes, you are right, it was wrong. However, I already deleted the bit7z.vcxproj
file from the repository, since maintaining multiple project files (qmake, MSBuild/Visual Studio, CMake) is not a good idea, and it is time consuming. Since now bit7z supports Linux, I will only maintain the CMake one, which is almost a de facto standard for cross-platform project configuration (even though I don't like its syntax, tbh).
- You are setting different output names for static libraries --
bit7z
,bit7z_d
,bit7z64
,bit7z64_d
. I am not sure whether that is required by some standard or not, but it just makes including them in another project harder because one has to manually specify all variations which is tedious and error prone. On the other hand, if the output name was the same, they could just put$(SolutionDir)bit7z\bin\$(PlaformTarget)\$(Configuration)\
to library path andbit7z.lib
to additional libraries for all configurations and all platforms at once and be done with it.
Yes, is a convention of many projects/platforms, even though with different variations (e.g., debug libraries of the Qt framework have d
suffix, instead of _d
).
An advantage of such convention is a less complex directory structure. Anyway, I will evaluate whether to change also this.
- Defines for
BIT7Z_AUTO_FORMAT
andBIT7Z_REGEX_MATCHING
should in my opinion be in the main bit7z include file. Rationale is that if I wantBIT7Z_REGEX_MATCHING
I need to put it in preprocessor directives in both bit7z project and in my own project to actually be able to use the methods. If it was defined in the header file (but commented out by default), I could just uncomment it before building and have it magically work in both projects. This is what other libraries usually do (i.e. the header files reflect build configuration).
The problem is... there is no "main" bit7z include file. If you mean bit7z.hpp
, this is not included by any other file in bit7z, hence defining those macros there would not have an effect (or at least, not if you use bit7z a static library, as it is supposed to be). The only header file that is used almost everywhere is bittypes.hpp
, so putting them there probably it would work, and I will evaluate such a possibility.
- Your 7zSDK folder is a subfolder of your bit7z project. I personally prefer when 3rd party code resides outside of the project tree -- say a solution folder named bit7z with project folder bit7z and 7zSDK folder at same level as the project. What I usually do is:
+-MyAwesomeSolution +-3rdParty +-7zSDK +-bit7z +-include +-lib +-x64 +-Debug +-Release +-x86 +-Debug +-Release +-OpenSSL +-toml11 +-readerwriterqueue +-rapidjson +-TurboJPEG +-MyAwesomeLibraryProject +-MyAwesomeExecutableProject
If I understand correctly, you would prefer a project structure like the following:
+- bit7z
+- 3rdparty
+- 7-zip
+- bit7z
+- include
+- lib
...
Personally, I don't see any advantage in a solution folder like this. Moreover, it requires a further nesting of what I think it really matters, i.e., the source code of bit7z. Probably, this kind of structure would be more useful for a Visual Studio solution (.sln + .vcxproj files), not a CMake project.
In this configuration everything is a separate project. In your case even 7zSDK could be a separate static library project. Rationale is that you don't need to rebuild projects which were not modified since the last build, which speeds up the build process.
As I already said, now bit7z uses only some headers and no source files from 7-zip (and only a single source file from p7zip). Probably, in the future bit7z will also support linking to a full static library version of 7-zip in order completely remove the dependency on its DLLs (and even optionally make bit7z a DLL itself). However, even in that case 7-zip will be considered as a separate project, though in a sub-folder of bit7z. Moreover, since updates to the code of 7-zip are quite infrequent, there will be no need to re-compile it every time.
Hence, at least for the moment I would prefer a more conservative approach and retain 7-zip in a "3rdparty" sub-folder of bit7z.
Finally, you are not using precompiled headers (i.e.
pch.h
,pch.cpp
). No idea why not, but they usually speed up the build considerably.
Yes, they do, but I got some issues with them in the past, so I preferred to not use them at all. However, I will certainly try again, since the project now has grown quite a bit and speeding up the compilation time can be useful.
To summarize my suggestions:
- Make a solution folder.
- Move 7zSDK to solution level under
3rdParty
folder (maybe you will have to add more 3rd party code later and putting it all under3rdParty
folder will reduce clutter).- Turn 7zSDK into a static library project.
- Change
OutDir
to$(ProjectDir)lib\$(PlatformTarget)\$(Configuration)\
.- Change
IntDir
to$(ProjectDir)obj\$(PlatformTarget)\$(Configuration)\
.- Change
TargetName
back to$(ProjectName)
.- Add
//#define BIT7Z_AUTO_FORMAT
and//#define BIT7Z_REGEX_MATCHING
tobit7z.hpp
- Add precompiled headers to bit7z project.
I am sure some of those won't be practical and easy to implement but hopefully you will consider at least some of my suggestions.
I want to thank you again for the suggestions you gave me. I wanted to somehow reorganize the project structure for a while now, and also thanks to your suggestions, I came to this new possible structure of the project:
+- bit7z
+- 3rdparty
+- 7-zip
+- p7zip
+- doc //documentation
+- include //public headers (e.g., .hpp files with "bit" prefix)
+- lib //output .lib files (possibly using the new naming convention you suggested, I still need to evaluate this more deeply)
+- src //source code and private headers (e.g., .hpp files without "bit" prefix)
...
CMakeLists.txt
...
This new structure should also simplify the CI configuration on AppVeyor. Obviously, I'm open to any suggestion or evaluation concerning this new scheme, which I will not introduce immediately anyway. I hope I have clarified your doubts and thank you again for the suggestions!
Hi,
Sorry for the late reply and thank you very much for your feedback
No problem, you are welcome.
Since now bit7z supports Linux, I will only maintain the CMake one
Have you considered providing an option to use CMake to generate .vcxproj
file for those who might need it?
An advantage of such convention is a less complex directory structure.
I am aware of that, but please consider that using such convention in projects relying on makefiles is much more trivial than going trough property pages for all combinations of Debug
, Release
, x86
, and x64
and setting eight different library filenames.
The problem is... there is no "main" bit7z include file.
Then I think it may be a good idea to add a bit7z_conf.hpp
with those macros, and include it only in source files which use said macros for enabling and disabling functionality. Then we can do:
#include "bit7z_conf.hpp"
#include "bit7z.hpp"
As already mentioned, I decided to put the library output files in
bin
since they are in fact binary files.
There are several points to consider here:
Linux distributions (and by extension Linux compilers) do not make a distinction between static and shared (a.k.a. dynamic) libraries in the way Windows does. Under Linux, both libc.a
and libc.so
are traditionally installed into the same system lib
folder. Also, any projects that produce both shared and static library put those into the same lib
folder. Under Windows, programs never put static libraries in C:\Windows\System32
and usually never install their shared libraries there, especially not during build process.
Under Linux, it is not usual to look for shared libraries in the application directory, under Windows that's the first place to check.
Under Linux, linking an executable to libc.so
is direct, ld
does not require additional intermediary files. Under Windows, linking to vcruntime140.dll
requires a shared vcruntime.lib
library.
With that in mind, on Windows, vcruntime.lib
is a binary file as you say, but just by looking at the .lib
extension you cannot know whether it is a static or a shared (a.k.a. export) library.
So, if your project produces both bit7z.lib
and bit7z.dll
, you will have two bit7z.lib
files, and on Windows it is quite common to output this shared/export bit7z.lib
into same folder as bit7z.dll
. If I am not mistaken that's the Visual Studio default. Now, if you also have an executable program in your solution, such as bit7ztest.exe
which is linking to bit7z.dll
so you can verify it is working correctly, this hypothetical bit7ztest.exe
won't start and you won't be able to debug it unless bit7z.dll
is in the same folder.
In my opinion, that leaves you only one viable option -- put bit7ztest.exe
into bin
folder, put bit7z.dll
and shared/export bit7z.lib
into bin
folder, and put static bit7z.lib
into lib
folder.
Sure, you can use different library names like some other open-source projects are doing, but that just complicates setup for anyone trying to use it.
If I understand correctly, you would prefer a project structure like the following
Yes.
Personally, I don't see any advantage in a solution folder like this
It allows you to add another project (say a hypothetical bit7ztest
application) in the future, and have a common build output at the solution folder level while keeping temporary object files separated per project folder.
I hope you are safe.
I wanted to include bit7z in one of my projects as a git submodule.
What feels weird to me (kind of a brain dump after spending some time integrating it with my stuff):
7zSDK
in thelib
subfolder of the project when those are actually source files, not prebuilt static library (or output static library) files.bin
folder which is usually reserved for input or output executables for projects that have both. You might want to make bit7z DLL in the future, and then where will yourbit7z.dll
and corresponding import libraries go if you are putting static libraries inbin
folder?OutDir
andIntDir
directives inbit7z.vcxproj
file contain fully relative paths instead of specifying$(ProjectDir)bin\
or$(ProjectDir)build\
.bit7z
,bit7z_d
,bit7z64
,bit7z64_d
. I am not sure whether that is required by some standard or not, but it just makes including them in another project harder because one has to manually specify all variations which is tedious and error prone. On the other hand, if the output name was the same, they could just put$(SolutionDir)bit7z\bin\$(PlaformTarget)\$(Configuration)\
to library path andbit7z.lib
to additional libraries for all configurations and all platforms at once and be done with it.BIT7Z_AUTO_FORMAT
andBIT7Z_REGEX_MATCHING
should in my opinion be in the main bit7z include file. Rationale is that if I wantBIT7Z_REGEX_MATCHING
I need to put it in preprocessor directives in both bit7z project and in my own project to actually be able to use the methods. If it was defined in the header file (but commented out by default), I could just uncomment it before building and have it magically work in both projects. This is what other libraries usually do (i.e. the header files reflect build configuration).In this configuration everything is a separate project. In your case even 7zSDK could be a separate static library project. Rationale is that you don't need to rebuild projects which were not modified since the last build, which speeds up the build process.
Finally, you are not using precompiled headers (i.e.
pch.h
,pch.cpp
). No idea why not, but they usually speed up the build considerably.To summarize my suggestions:
3rdParty
folder (maybe you will have to add more 3rd party code later and putting it all under3rdParty
folder will reduce clutter).OutDir
to$(ProjectDir)lib\$(PlatformTarget)\$(Configuration)\
.IntDir
to$(ProjectDir)obj\$(PlatformTarget)\$(Configuration)\
.TargetName
back to$(ProjectName)
.//#define BIT7Z_AUTO_FORMAT
and//#define BIT7Z_REGEX_MATCHING
tobit7z.hpp
I am sure some of those won't be practical and easy to implement but hopefully you will consider at least some of my suggestions.