ENABLE_DEBUG_LOG is ignored on Windows

[zethon]

The documentation says:

    *  _MSC_VER Only - ENABLE_DEBUG_LOG is automatically enabled in the Debug
    *  build, unless DISABLE_DEBUG_LOG is defined. See 'tidyplatform.h'

And there seems to be no way to disable other than defining DISABLE_DEBUG_LOG. This seems bizarre. Why shouldn't ENABLE_DEBUG_LOG=OFF be honored on all platforms?

[zethon]

I've also yet to figure out a way to define DISABLE_DEBUG_LOG on the CMake command line (or via a conan recipe). If the CMake variable was honored this would not only make more sense but be a lot easier!

[geoffmcl]

@zethon thank you for the issue... but unfortunately, do not see the problem here... sorry...

As a maintainer, I have used this auto feature, over many, MANY years, as my first view of a bug... it has helped countless times... and is always a good reminder of a files html progress through libTidy...

So *IFF you are building the MSVC DEBUG configuration of tidy, to check a bug, then why would you not want this extra help? Please explain...

Let's look at the auto enabling code, in tidyplatform.h -

#if defined(_WIN32) && !defined(__MSL__) && !defined(__BORLANDC__)
...
# if defined(_MSC_VER)
...
#if !defined(NDEBUG) && !defined(ENABLE_DEBUG_LOG) && !defined(DISABLE_DEBUG_LOG)
#define ENABLE_DEBUG_LOG
...

Essentially, that is ONLY IF (_WIN32 AND _MSC_VER, AND NOT NDEBUG)...

So this restricts it to ONLY the MSVC DEBUG configuration build...

Other configuration, like Release MinSizeRel RelWithDebInfo ..., all define NDEBUG, so would be excluded...

Unless, you, for some reason, wanted it ON in ALL MSVC configurations... especially in a Release... that I would find bizarre...

And a person desiring to actually DEBUG Tidy, in the MSVC IDE, can easily drop in a #define DISABLE_DEBUG_LOG, or comment out the //#define ENABLE_DEBUG_LOG, or ... if they feel this is bizarre...

In ALL other cases, the CMake variable is honored...

At the moment this verges on Won't fix... without further feedback, comments, explanations, reasons... thanks...

[zethon]

So *IFF you are building the MSVC DEBUG configuration of tidy, to check a bug, then why would you not want this extra help? Please explain...

Wanting to run my app in Debug mode without the debug output of the libraries it's using is a very reasonable use case. If I was building tidy-html5 alongside my project it would be straightforward to accomplish, however the consuming project is not always built with the libraries. And if I link my Debug application to a non-Debug build of tidy-html5 then I will get linking errors (or at least warnings).

In my case I am building tidy-html5 through a conan recipe, so by the time I'm building my project, the .lib has already been built. What brought me to write this Issue was that the CMake setting ENABLE_DEBUG_LOG was sitting right there staring at me, and when I went to use it, it didn't work. The more I read about why it didn't work was when I discovered (what you already know, of course) is that the debug log is always on on Windows unless you use a preprocessor definition. This seems inconsistent and hence my use of the word "bizarre" (I meant no offense).

Of course, if I could find a reasonable way to pass a preprocessor definition to MSBuild through CMake (and ultimately conan), then it wouldn't matter. But that is another story.

Thank you for your time.

[zethon]

NOTE: In case anyone in the future stumbles upon this issue of trying to pass a preprocessor definition to MSBuild (which is what ultimately brought me here), I was able to accomplish that by setting an environmental variable that MSBuild reads. See: zethon/conan-tidy-html5@20e86eb#diff-2b1d42f71f22b7ea0412d7602dec166f

But I still think the CMake setting ENABLE_DEBUG_LOG=Off should be honored. :)

[geoffmcl]

@zethon, sorry if I came across too strong... sorry...

Yes, agree linking an App to libTidy is valid use case...

Where such a DEBUG version of the library, like the static tidysd.lib, or the shared/DLL tidyd.lib... would not be needed...

In this case, like many other 3rdParty libraries, only the Release should be installed... and found in any normal dependency search...

You suggest link warning/error... What are they? ...

It seems very common to only install the Release version... although there are a few exceptions, here and there...

I also work on several projects, that link to libtidy...

In one, edbrowse, I can say I was surprised the first time, running MSVC Debug, on it, to see this additional output... because I had allowed it to find and link to both tidy configs... log extract -

-- +++ Search using paths F:/Projects/software.x64
-- *** Tidy found inc F:/Projects/software.x64/include lib 
   debug;F:/Projects/software.x64/lib/tidysd.lib;optimized;F:/Projects/software.x64/lib/tidys.lib

But it allowed me to see when the app ran libTidy, and see what it was getting back... etc ... so eventually accepted it, as additional debug information...

Just moved/renamed the installed tidysd.lib, deleted CMakeCache.txt, and the project re-built fine... now only finds tidys.lib... no debug...

I do get a Debug link warning LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts with use of other libs; use /NODEFAULTLIB:library, but I think that was there before... or...

At least I do see it many times, and have learned to ignore it... perhaps to my peril ;=))

In other projects, I also accept the additional tidysd.lib output... does no harm... adds info...

Anyway, glad you found the CL environment var, for MSBuild/CL compiler... must try that...

But otherwise, what CMakeLists.txt changes would you suggest?

Aside from the obvious, repeated, the CMake setting ENABLE_DEBUG_LOG=Off should be honored. ;=))

Which just can not work for multible configs...

This maybe, sort of started with commit dedcb7b, and several subsequent, important additions/changes... maybe I missed it there...

I/We need a way to enable the log, but ONLY in the MSVC Debug build... it should not be on, for all configs, especially. for, Release, et al...

And remember, this only applies to cmake generatered, buld files, that support multiple configs, in one build set...

It does not effect, say, single config systems, like say unix Makefiles, etc...

Is it worth another cmake option, like, if (MSVC), DISABLE_DEBUG_LOG, On, Off(def), then -

if (DISABLE_DEBUG_LOG)
    add_definitions( -DDISABLE_DEBUG_LOG )
endif ()

I'm presently abt 50:50 on this... but could be convinced of its importance...

Especially after trying, experimenting, testing... the set CL=/DDISABLE_DEBUG_LOG=1 option... now offered... but if it works... 49:51...

As well as the obvious advice: Do NOT link your app to the tidy WIN32 MSVC DEBUG library! .. a docs addition, somewhere...

Use the installed Release instead... static or shared... as you would do with most other 3rdParty library dependencies... libTidy is not different...

Look forward to further feedback, comments, patches, PR, etc... What do others think?... thanks...

[zethon]

I would have to go back and link a Debug build against a Release build of libtidy to remember what linking warnings I was seeing, but LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts with use of other libs; use /NODEFAULTLIB:library warning sounds very familiar. It may be completely harmless to just ignore any linking errors I in the Debug/Release mix case, I just like striving for warningless builds. :)

Is it worth another cmake option, like, if (MSVC), DISABLE_DEBUG_LOG, On, Off(def), then -

I feel like that would be confusing given ENABLE_DEBUG_LOG exists. I still don't quite understand the issue with having ENABLE_DEBUG_LOG=OFF honored on a Windows Debug build, but my history/context with tidy-html as a project is very limited.

Still though, I would be very happy for any CMake solution you implement. My set CL solution does work but it feels very hackish.

Thanks.

[geoffmcl]

@zethon thanks for the further feedback...

I too strive for a warningless build ;=)) always the best...

But I now see this as just not a use case...

You choose to link your app to an uninstalled DEBUG MSVC version of libTidy... no particular problem with that... go for it... enjoy... it is good to debug 2, or more, things, at the same time, as I run the MSVC IDE Debug...

Then do not like the extra debug infomation provided by the tidy debug lib, auto enabled ONLY in the MSVC debug build... although there are ways to avoid even this... hmmm...

My question is still - how to enable the LOG in the Debug build, while NOT enabling it in the other builds? And avoid multiple build trees, that would be necessary for any singular build system, like Unix Makefile...

For more than 5+ years, I used auto process currently in place... it is useful... for the reasons stated...

This is the first comment about that... thanks for noting... I think... but is it, it does not seem an ISSUE...

Talking about hacking...

You could also add int add_std_out(int);void set_log_file(char *, int ); to your apps MSVC Debug build, and add add_std_out(0);set_log_file("none",0); before you use the libTidy...

That would certainly be called hackish ;=)) but would certainly work!

So if you can answer how to have ENABLE_DEBUG_LOG=ON, in the cmake config process, to only be honored, in MSVC Windows Debug build, in CMake terms, then I am happy to listen... learn...

At this moment I do not have the solution that fits ALL... have tried, hard... and continue to listen...

But, every time you have the need to enable/disable some option, be it in cmake, environment, where ever... terms... to get the build you want...

I do not call it a hack... it is exercising a user option...

IFF there is a problem linking with the installed, release version of libTidy, then for sure, this should/could be addressed in a separate, new issue...

So as yet, I do not have a CMake solution for this... any help with that much appreciated...

Further feedback, comments, patches, PR, issues, etc... appreciated... thanks...

[geoffmcl]

20200923: @zethon as indicated I do not see an open issue here... so closing this...

But thanks for raising it, and the discussions... and hope you are settled on how to use libTidy in your projects...

As always, look forward to further feedback, comments, patches, PR, issues, etc... appreciated... thanks...