MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003280Doomseeker[All Projects] Suggestionpublic2017-09-27 23:152021-12-09 01:15
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritylowSeveritytweakReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.1 
Target Version1.3Fixed in Version1.3 
Summary0003280: Make building Doomseeker and libwadseeker as independent steps feasible
DescriptionCurrently, libwadseeker can already be built standalone but building Doomseeker from source also requires building libwadseeker (or Doomseeker doesn't use the system libraries for libwadseeker).

Documentation for building Doomseeker standalone is also missing.
Steps To Reproduce
Additional InformationOriginally suggested by Blzut3 in issue ticket 0003238 comment 0018331.

Quote from Blzut3
I'm also open to looking into making sure that it is feasible to build wadseeker and doomseeker as independent steps (while wadseeker should be able to build standalone, I don't think we have any way to use a system copy of wadseeker when building doomseeker). This should mostly be a matter of making sure our CMake usage is idiomatic anyway.
TagsNo tags attached.
Relationships
related to 0003931new  libwadseeker-dev 2.1: Using internal Wadseeker library 
child of 0003246acknowledged  Debian packaging 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2017-09-27 23:15WubTheCaptainNew Issue
2017-09-27 23:17WubTheCaptainRelationship addedchild of 0003246
2017-10-05 01:47WubTheCaptainStatusnew => confirmed
2018-10-05 07:40WubTheCaptainPrioritynormal => low
2019-01-17 15:16Pol MAssigned To => Pol M
2019-01-17 15:16Pol MStatusconfirmed => assigned
2019-01-17 17:54Pol MNote Added: 0020313
2019-01-17 17:54Pol MStatusassigned => needs review
2019-01-18 00:30WubTheCaptainNote Added: 0020315
2019-01-18 00:30WubTheCaptainStatusneeds review => assigned
2019-01-18 00:33WubTheCaptainTarget Version => 1.2
2019-01-18 00:35WubTheCaptainTarget Version1.2 => 1.3
2019-01-18 06:49Pol MNote Added: 0020316
2019-01-18 15:46Pol MNote Edited: 0020316bug_revision_view_page.php?bugnote_id=20316#r12360
2019-01-18 19:56Pol MStatusassigned => needs review
2019-01-18 20:26Pol MStatusneeds review => assigned
2019-01-26 20:16Pol MStatusassigned => needs review
2019-02-05 20:28ZalewaAssigned ToPol M => Zalewa
2019-02-05 20:28ZalewaStatusneeds review => assigned
2019-02-05 20:29ZalewaNote Added: 0020338
2019-02-05 20:30ZalewaNote Edited: 0020338bug_revision_view_page.php?bugnote_id=20338#r12391
2019-02-05 20:30ZalewaNote Edited: 0020338bug_revision_view_page.php?bugnote_id=20338#r12392
2019-02-11 15:22WubTheCaptainNote Added: 0020368
2019-02-11 15:41WubTheCaptainNote Added: 0020369
2019-02-11 15:42WubTheCaptainNote Added: 0020370
2019-02-11 15:43WubTheCaptainNote Added: 0020371
2019-02-11 16:26Pol MNote Added: 0020374
2019-02-11 16:34Pol MNote Edited: 0020374bug_revision_view_page.php?bugnote_id=20374#r12413
2019-02-11 16:36Pol MNote Edited: 0020374bug_revision_view_page.php?bugnote_id=20374#r12414
2019-02-11 16:49Pol MNote Edited: 0020374bug_revision_view_page.php?bugnote_id=20374#r12415
2019-02-11 17:04WubTheCaptainNote Added: 0020376
2019-02-11 17:05WubTheCaptainNote Edited: 0020376bug_revision_view_page.php?bugnote_id=20376#r12417
2019-02-11 17:19Pol MNote Added: 0020377
2019-02-11 17:39WubTheCaptainNote Added: 0020378
2019-02-11 17:42WubTheCaptainNote Edited: 0020378bug_revision_view_page.php?bugnote_id=20378#r12419
2019-02-11 17:43WubTheCaptainNote Edited: 0020378bug_revision_view_page.php?bugnote_id=20378#r12420
2019-02-11 17:49WubTheCaptainNote Added: 0020379
2019-02-11 17:49WubTheCaptainNote Edited: 0020379bug_revision_view_page.php?bugnote_id=20379#r12422
2019-02-11 17:53Pol MNote Added: 0020380
2019-02-11 17:55Pol MNote Edited: 0020380bug_revision_view_page.php?bugnote_id=20380#r12424
2019-06-22 09:47ZalewaNote Added: 0020800
2019-06-22 09:47ZalewaAssigned ToZalewa =>
2019-06-22 09:47ZalewaStatusassigned => confirmed
2019-06-22 09:51ZalewaNote Edited: 0020800bug_revision_view_page.php?bugnote_id=20800#r12669
2019-06-22 10:05ZalewaNote Added: 0020801
2019-06-24 16:00Pol MNote Added: 0020826
2019-06-24 16:01Pol MAssigned To => Pol M
2019-06-24 16:01Pol MStatusconfirmed => assigned
2019-06-24 16:03Pol MNote Edited: 0020826bug_revision_view_page.php?bugnote_id=20826#r12690
2019-06-24 18:03ZalewaNote Added: 0020828
2019-06-24 18:33Pol MNote Added: 0020829
2019-06-24 19:32Pol MNote Edited: 0020829bug_revision_view_page.php?bugnote_id=20829#r12692
2019-06-24 20:48Pol MNote Edited: 0020829bug_revision_view_page.php?bugnote_id=20829#r12693
2019-06-24 20:49Pol MAssigned ToPol M =>
2019-06-24 20:49Pol MStatusassigned => confirmed
2019-06-26 11:37Pol MNote Edited: 0020829bug_revision_view_page.php?bugnote_id=20829#r12699
2019-06-26 11:38Pol MAssigned To => Pol M
2019-06-26 11:38Pol MStatusconfirmed => assigned
2019-06-26 11:38Pol MStatusassigned => needs review
2019-06-30 08:44ZalewaAssigned ToPol M => Zalewa
2019-06-30 08:44ZalewaStatusneeds review => assigned
2019-06-30 08:47ZalewaNote Added: 0020847
2019-06-30 09:20ZalewaNote Edited: 0020847bug_revision_view_page.php?bugnote_id=20847#r12706
2019-06-30 09:21ZalewaStatusassigned => needs testing
2019-07-10 05:55ZalewaNote Added: 0020869
2019-07-10 05:55ZalewaStatusneeds testing => resolved
2019-07-10 05:55ZalewaFixed in Version => 1.3
2019-07-10 05:55ZalewaResolutionopen => fixed
2019-07-30 10:13WubTheCaptainStatusresolved => closed
2021-12-09 01:15WubTheCaptainRelationship addedrelated to 0003931

Notes
(0020313)
Pol M   
2019-01-17 17:54   
pr
(0020315)
WubTheCaptain   
2019-01-18 00:30   
Quote from releasescripts/makesourcepackages.sh:147
echo 'Adding lincence and dependencies for independent building...'


license
(0020316)
Pol M   
2019-01-18 06:49   
(edited on: 2019-01-18 15:46)
Ouch. Will fix today 😅😂
(Done!)
(0020338)
Zalewa   
2019-02-05 20:29   
(edited on: 2019-02-05 20:30)
PR is merged, so Linux side of things should be done. I'll try to make this work on Windows too.
(0020368)
WubTheCaptain   
2019-02-11 15:22   
Quote from CMakeLists.txt
if ( wadseeker_FOUND AND NOT FORCE_INTERNAL_WADSEEKER )
    message( STATUS "Using system Wadseeker library" )
else( wadseeker_FOUND AND NOT FORCE_INTERNAL_WADSEEKER )
    message( STATUS "Using internal Wadseeker library" )


What is the difference?
(0020369)
WubTheCaptain   
2019-02-11 15:41   
Quote from makesourcepackages.sh
	echo 'Adding licence and dependencies for independent building...'
    cp "$doomseekerArchiveName/LICENSE.json" "$doomseekerArchiveName/src/wadseeker"
    
cp -r "$doomseekerArchiveName/cmake" "$doomseekerArchiveName/src/wadseeker"


I don't use this script, but it could be fragile if wadseeker directory is missing. (LICENSE.json would be renamed as wadseeker, and/or cmake would be renamed as wadseeker directory).
(0020370)
WubTheCaptain   
2019-02-11 15:42   
Quote
wadseeker_FOUND


WADSEEKER_FOUND for consistency?
(0020371)
WubTheCaptain   
2019-02-11 15:43   
Quote
find_library_wadseeker


find_package_WADSEEKER for consistency?
(0020374)
Pol M   
2019-02-11 16:26   
(edited on: 2019-02-11 16:49)
Quote from Wub

What is the difference?

It prints whether we are using wadseeker as an internal library or if we are using the system's library. The content in the else (i think) is optional. There is nothing wrong here.

Quote from Linda

I don't use this script, but it could be fragile if wadseeker directory is missing. (LICENSE.json would be renamed as wadseeker, and/or cmake would be renamed as wadseeker directory).

This script creates the source files, it is intended to be used with the entire source directory. The lines you quoted simply add the licence into wadseeker since now it can be built independently (and we should also distribute it with the source of wadseeker because if not it won't be able to install the licence with the rest of the library, as we do currently), and also it copies some useful tools for cmake.
There is nothing wrong so far.

Quote from Wub

find_package_WADSEEKER for consistency?

I'd agree on renaming it to "find_package_wadseeker". The name used should be the same as in "find_package(wadseeker CONFIG ${ARGV0})". note that to find it, we are using "wadseeker".

I'll bundle all changes in one commit.

Quote from Wub

WADSEEKER_FOUND for consistency?

We do not set this variable.

EDIT: pr
(0020376)
WubTheCaptain   
2019-02-11 17:04   
(edited on: 2019-02-11 17:05)
Quote from Pol M
It prints whether we are using wadseeker as an internal library or if we are using the system's library.


I'm still confused, the conditions are still the same.

if (a)
    # do something with a
else (a)
    # also a; never reached?


(0020377)
Pol M   
2019-02-11 17:19   
Cmake uses if/elseif/else. The else block will be reached if the rest of the conditions are not met. You can see an example usage in the cmake documentation, and it also says there that the condition is optional. So, this:

if(expression)
  # if block.
else(expression)
  # else block.

is the same as:

if(expression)
  # if block.
else()
  # else block.
(0020378)
WubTheCaptain   
2019-02-11 17:39   
(edited on: 2019-02-11 17:43)
That's now how the latest version (currently 3.14.0-rc1) describes it:

Quote from https://cmake.org/cmake/help/latest/command/if.html
if(<condition>)
  <commands>
elseif(<condition>) # optional block, can be repeated
  <commands>
else()              # optional block
  <commands>
endif()


It's confusing or possibly misleading, so CMake developers have also fixed it:'https://github.com/Kitware/CMake/commit/c2efb3efcd083523a73a2a9721b7101fbfc0fe0f#diff-753915a5d44fe7ebf5ef8182577262ff [^]'

The legacy behavior will work:

Quote from https://cmake.org/cmake/help/latest/command/if.html
Per legacy, the else() and elseif() commands admit an optional <condition> argument. If used, it must be a verbatim repeat of the argument of the opening if command.


but I don't see any sane person ever doing that.
(0020379)
WubTheCaptain   
2019-02-11 17:49   
Quote from https://cmake.org/cmake/help/latest/command/if.html
If the result is true, then the commands in the if block are executed. Otherwise, optional elseif blocks are processed in the same way.

Per legacy, the else() and elseif() commands admit an optional <condition> argument. If used, it must be a verbatim repeat of the argument of the opening if command.


I also see a contradiction here about elseif().
(0020380)
Pol M   
2019-02-11 17:53   
(edited on: 2019-02-11 17:55)
Correct. In this case, it is used to blend with the code surrounding those lines. As you said, it is still correct to use the if condition in the else, and I prioritized making code that blends in, making it easier to read as a consequence.

I also agree that it is not intuitive to use a condition into an else.

Quote from Wub

I also see a contradiction here about elseif().

Yeah, me too :)
(0020800)
Zalewa   
2019-06-22 09:47   
(edited on: 2019-06-22 09:51)
I started working on getting this to work on Windows and I discovered that it doesn't even work on Linux.

1. I exported libwadseeker source archive with the "makesourcepackages.sh" script.
2. Compiled it, then "make install" to a directory of my choice.
3. Went to compiling Doomseeker from the root CMakeLists.txt
4. Disabled the FORCE_INTERNAL_WADSEEKER, pointed Wadseeker_DIR to the location where the "WadseekerConfig.cmake" was created.
5. Discovered that the external Wadseeker is not being detected because it expects "wadseekerConfig.cmake".
6. Corrected that error.
7. Discovered that the #include path is resolved to "${wadseeker-install-dir}/include/wadseeker" whereas it should be "${wadseeker-install-dir}/include"
(0020801)
Zalewa   
2019-06-22 10:05   
And I also had to revert the correction commit that made "Wadseeker" naturally capitalized because it caused compilation problems on Windows due to #ifdef wadseeker_EXPORTS in wadseekerexportinfo.h.
(0020826)
Pol M   
2019-06-24 16:00   
(edited on: 2019-06-24 16:03)
I do not know how it even worked when I pushed the project, I'll work on this asap.
EDIT: Ok, got it to work on Linux by specifying the project name in lowercase instead of "Wadseeker" and with a small touch here and there, I'll push and immediately I'll work on getting it to work on Windows.
(0020828)
Zalewa   
2019-06-24 18:03   
It would be preferable not to change the project name unless the current name causes violations of some standards. Note that CMake built-in "Find" modules define the targets using the natural capitalization: Freetype, OpenGL, Qt, etc. Hence it appears that "Doomseeker" and "Wadseeker" are correct names for the project() statement.
(0020829)
Pol M   
2019-06-24 18:33   
(edited on: 2019-06-26 11:37)
Sorry, I expressed myself poorly. I meant to say that instead of using the project name, in some locations I should have used the TARGETS name, which is "wadseeker".
EDIT: aka. The project name hasn't changed.
EDIT2: PR I did not manage to make it work on Windows :( I included some changes to solve some errors I encountered on Windows.
I did it! YAY!
(0020847)
Zalewa   
2019-06-30 08:47   
(edited on: 2019-06-30 09:20)
Excellent. I tested the commits and it all seems to work correctly now. Much thanks go for your effort. I rebased the PR, kept the commit history and rephrased the commit messages so that they explain in more detail what each commit does:

*'https://bitbucket.org/Doomseeker/doomseeker/commits/51fdd3cbad1025ac099f81c3d1dc5c7599db19a7 [^]'
*'https://bitbucket.org/Doomseeker/doomseeker/commits/305fca2af65546c0946417d672f02ab38253a95a [^]'
*'https://bitbucket.org/Doomseeker/doomseeker/commits/abd9b9e39c2383e9298e928a7c600837fac8c426 [^]'
*'https://bitbucket.org/Doomseeker/doomseeker/commits/c33f6f5d59603e7481cd89d1de410349635b0f11 [^]'

An additional bug in #include statements of Wadseeker's exported headers was found:'https://bitbucket.org/Doomseeker/doomseeker/commits/1eed1f64089fdfa632d81f63900163b5d1fe87d7 [^]'

And the WadseekerApp example tool is brought up-to-date here:'https://bitbucket.org/Doomseeker/doomseeker/commits/ef3677b24b5808c840afa1c028c6a5a32f26a98d [^]'

I'm putting this in "needs testing" state now although I think that it works.
(0020869)
Zalewa   
2019-07-10 05:55   
It works.