Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003280Doomseeker[All Projects] Suggestionpublic2017-09-27 23:152019-07-10 05:55
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritylowSeveritytweakReproducibilityhave not tried
StatusresolvedResolutionfixed 
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.
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.
Attached Files

- Relationships
child of 0003246confirmedWubTheCaptain Debian packaging 

-  Notes
User avatar (0020313)
Pol M (developer)
2019-01-17 17:54

pr
User avatar (0020315)
WubTheCaptain (developer)
2019-01-18 00:30

Quote from releasescripts/makesourcepackages.sh:147
echo 'Adding lincence and dependencies for independent building...'


license
User avatar (0020316)
Pol M (developer)
2019-01-18 06:49
edited on: 2019-01-18 15:46

Ouch. Will fix today 😅😂
(Done!)

User avatar (0020338)
Zalewa (developer)
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.

User avatar (0020368)
WubTheCaptain (developer)
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?
User avatar (0020369)
WubTheCaptain (developer)
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).
User avatar (0020370)
WubTheCaptain (developer)
2019-02-11 15:42

Quote
wadseeker_FOUND


WADSEEKER_FOUND for consistency?
User avatar (0020371)
WubTheCaptain (developer)
2019-02-11 15:43

Quote
find_library_wadseeker


find_package_WADSEEKER for consistency?
User avatar (0020374)
Pol M (developer)
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

User avatar (0020376)
WubTheCaptain (developer)
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? 


User avatar (0020377)
Pol M (developer)
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.
User avatar (0020378)
WubTheCaptain (developer)
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.

User avatar (0020379)
WubTheCaptain (developer)
2019-02-11 17:49
edited on: 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().

User avatar (0020380)
Pol M (developer)
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 :)

User avatar (0020800)
Zalewa (developer)
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"

User avatar (0020801)
Zalewa (developer)
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.
User avatar (0020826)
Pol M (developer)
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.

User avatar (0020828)
Zalewa (developer)
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.
User avatar (0020829)
Pol M (developer)
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!

User avatar (0020847)
Zalewa (developer)
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.

User avatar (0020869)
Zalewa (developer)
2019-07-10 05:55

It works.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: Pol M
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2017-09-27 23:15 WubTheCaptain New Issue
2017-09-27 23:17 WubTheCaptain Relationship added child of 0003246
2017-10-05 01:47 WubTheCaptain Status new => confirmed
2018-10-05 07:40 WubTheCaptain Priority normal => low
2019-01-17 15:16 Pol M Assigned To => Pol M
2019-01-17 15:16 Pol M Status confirmed => assigned
2019-01-17 17:54 Pol M Note Added: 0020313
2019-01-17 17:54 Pol M Status assigned => needs review
2019-01-18 00:30 WubTheCaptain Note Added: 0020315
2019-01-18 00:30 WubTheCaptain Status needs review => assigned
2019-01-18 00:33 WubTheCaptain Target Version => 1.2
2019-01-18 00:35 WubTheCaptain Target Version 1.2 => 1.3
2019-01-18 06:49 Pol M Note Added: 0020316
2019-01-18 15:46 Pol M Note Edited: 0020316 View Revisions
2019-01-18 19:56 Pol M Status assigned => needs review
2019-01-18 20:26 Pol M Status needs review => assigned
2019-01-26 20:16 Pol M Status assigned => needs review
2019-02-05 20:28 Zalewa Assigned To Pol M => Zalewa
2019-02-05 20:28 Zalewa Status needs review => assigned
2019-02-05 20:29 Zalewa Note Added: 0020338
2019-02-05 20:30 Zalewa Note Edited: 0020338 View Revisions
2019-02-05 20:30 Zalewa Note Edited: 0020338 View Revisions
2019-02-11 15:22 WubTheCaptain Note Added: 0020368
2019-02-11 15:41 WubTheCaptain Note Added: 0020369
2019-02-11 15:42 WubTheCaptain Note Added: 0020370
2019-02-11 15:43 WubTheCaptain Note Added: 0020371
2019-02-11 16:26 Pol M Note Added: 0020374
2019-02-11 16:34 Pol M Note Edited: 0020374 View Revisions
2019-02-11 16:36 Pol M Note Edited: 0020374 View Revisions
2019-02-11 16:49 Pol M Note Edited: 0020374 View Revisions
2019-02-11 17:04 WubTheCaptain Note Added: 0020376
2019-02-11 17:05 WubTheCaptain Note Edited: 0020376 View Revisions
2019-02-11 17:19 Pol M Note Added: 0020377
2019-02-11 17:39 WubTheCaptain Note Added: 0020378
2019-02-11 17:42 WubTheCaptain Note Edited: 0020378 View Revisions
2019-02-11 17:43 WubTheCaptain Note Edited: 0020378 View Revisions
2019-02-11 17:49 WubTheCaptain Note Added: 0020379
2019-02-11 17:49 WubTheCaptain Note Edited: 0020379 View Revisions
2019-02-11 17:53 Pol M Note Added: 0020380
2019-02-11 17:55 Pol M Note Edited: 0020380 View Revisions
2019-06-22 09:47 Zalewa Note Added: 0020800
2019-06-22 09:47 Zalewa Assigned To Zalewa =>
2019-06-22 09:47 Zalewa Status assigned => confirmed
2019-06-22 09:51 Zalewa Note Edited: 0020800 View Revisions
2019-06-22 10:05 Zalewa Note Added: 0020801
2019-06-24 16:00 Pol M Note Added: 0020826
2019-06-24 16:01 Pol M Assigned To => Pol M
2019-06-24 16:01 Pol M Status confirmed => assigned
2019-06-24 16:03 Pol M Note Edited: 0020826 View Revisions
2019-06-24 18:03 Zalewa Note Added: 0020828
2019-06-24 18:33 Pol M Note Added: 0020829
2019-06-24 19:32 Pol M Note Edited: 0020829 View Revisions
2019-06-24 20:48 Pol M Note Edited: 0020829 View Revisions
2019-06-24 20:49 Pol M Assigned To Pol M =>
2019-06-24 20:49 Pol M Status assigned => confirmed
2019-06-26 11:37 Pol M Note Edited: 0020829 View Revisions
2019-06-26 11:38 Pol M Assigned To => Pol M
2019-06-26 11:38 Pol M Status confirmed => assigned
2019-06-26 11:38 Pol M Status assigned => needs review
2019-06-30 08:44 Zalewa Assigned To Pol M => Zalewa
2019-06-30 08:44 Zalewa Status needs review => assigned
2019-06-30 08:47 Zalewa Note Added: 0020847
2019-06-30 09:20 Zalewa Note Edited: 0020847 View Revisions
2019-06-30 09:21 Zalewa Status assigned => needs testing
2019-07-10 05:55 Zalewa Note Added: 0020869
2019-07-10 05:55 Zalewa Status needs testing => resolved
2019-07-10 05:55 Zalewa Fixed in Version => 1.3
2019-07-10 05:55 Zalewa Resolution open => fixed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker