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-02-11 19:21
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritylowSeveritytweakReproducibilityhave not tried
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version1.1 
Target Version1.3Fixed in Version 
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 :)


Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
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






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker