[wtl] Remove 'wtl' subdirectory for upstream conformance

[JackBoosY]

The path to the internal files should add the wtl intermediate folder.

The installed header files that contain "#include <atl" are:

• wtl/atlapp.h:

#include <atlstr.h>

• wtl/atlapp.h:

 #include <atlcom.h>

• wtl/atlmisc.h:

 #include <atlstr.h>
 #include <atltypes.h>

• wtl/atlribbon.h:

#include <atlmisc.h>    // for RecentDocumentList classes
#include <atlframe.h>   // for Frame and UpdateUI classes
#include <atlctrls.h>   // required for atlctrlw.h
#include <atlctrlw.h>   // for CCommandBarCtrl

• wtl/atlwinx.h:

#include <atlwin.h>

However only wtl/atlribbon.h deals with this issue.

Fixes #24933.

Already tested the usage.

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

• ports/wtl/vcpkg.json

Valid values for the license field can be found in the documentation

[chrullrich]

This is the wrong approach. The correct one is to change #include <atl...> to #include "atl..." in atlribbon.h, i.e. what I tried to do in PR #25083 yesterday. I have already submitted that change upstream and expect it to be merged there.

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

• ports/wtl/vcpkg.json

Valid values for the license field can be found in the documentation

[JackBoosY]

@chrullrich This is not an upstream bug. Upstream expects that these header files should be installed directly in the folder include, so a direct include is valid. So this is a problem with vcpkg adaptation.
Since these files will not be installed by the Visual Studio ATL components, using < is also valid.

[chrullrich]

I don't see what VS has to do with this, and including a header you know to be in the same directory using < is a bug, or at least it is flirting with trouble.

[JackBoosY]

I don't see what VS has to do with this, and including a header you know to be in the same directory using < is a bug, or at least it is flirting with trouble.

1. wtl is a separate component from ATL, so the compiler will never find them in the include of Visual Studio ATL components.
2. Using "" to include in the same folder is valid. But you can also use <> in relative paths based on include directories.

The revisions to this PR are done based on the minimal change guidelines.

[chrullrich]

I don't see what VS has to do with this, and including a header you know to be in the same directory using < is a bug, or at least it is flirting with trouble.

1. wtl is a separate component from ATL, so the compiler will never find them in the include of Visual Studio ATL components.

I still don't get it, unless you are saying that since ATL happens not to include headers with colliding names (and certainly does not after patching in the wtl/, looking through the < include path is superior to going straight for the correct files in the same directory. It will not come as a great surprise that I disagree with that.

2. Using "" to include in the same folder is valid. But you can also use <> in relative paths based on include directories.

The revisions to this PR are done based on the minimal change guidelines.

Is that “When making changes to a library, strive to minimize the final diff”? Changing eight characters seems more minimal to me than adding 16.

Finally, and I admit this is not a strong argument (or even really relevant to vcpkg): Any (existing) code that does not use vcpkg's build system integration, e.g. with a Makefile that says "INCLUDE=...\wtl", will not work with the "wtl/" patched in.

[JackBoosY]

I still don't get it, unless you are saying that since ATL happens not to include headers with colliding names (and certainly does not after patching in the wtl/, looking through the < include path is superior to going straight for the correct files in the same directory. It will not come as a great surprise that I disagree with that.

ATL include path is VS_PATH/VC/Tools/MSVC/${TOOLSET_VERSION}/atlmfc/include, and I checked the header names: there is no conflict files.
See official document:

WTL is not included with Visual C++ but is made available both on the SourceForge Web site at [sourceforge.net/projects/wtl](https://sourceforge.net/projects/wtl) and through the Microsoft Download Center.

Is that “When making changes to a library, strive to minimize the final diff”? Changing eight characters seems more minimal to me than adding 16.

The "minimal changes" are for modifications in vcpkg, not for source code.
If we change the angle brackets to double quotes, we have to pick one of two modifications:

• Make a source patch. The size of this patch is significantly larger than one line of code.
• Read this header file, use string(REGEX REPLACE) to match and replace angle brackets with double quotes, and finally write to the file. This requires at least three lines of code.

Finally, and I admit this is not a strong argument (or even really relevant to vcpkg): Any (existing) code that does not use vcpkg's build system integration, e.g. with a Makefile that says "INCLUDE=...\wtl", will not work with the "wtl/" patched in.

You have to agree that any user who wants to use this port needs to add VCPKG_ROOT/installed/TRIPLET/include to their list of include directories. Therefore, there will be no errors with this modification.

[chrullrich]

ATL include path is VS_PATH/VC/Tools/MSVC/${TOOLSET_VERSION}/atlmfc/include, and I checked the header names: there is no conflict files.

I must be slow today because I still don't understand your point. Can we leave it be, please?

Is that “When making changes to a library, strive to minimize the final diff”? Changing eight characters seems more minimal to me than adding 16.

The "minimal changes" are for modifications in vcpkg, not for source code.

Then I don't understand this one either.

Finally, and I admit this is not a strong argument (or even really relevant to vcpkg): Any (existing) code that does not use vcpkg's build system integration, e.g. with a Makefile that says "INCLUDE=...\wtl", will not work with the "wtl/" patched in.

You have to agree that any user who wants to use this port needs to add VCPKG_ROOT/installed/TRIPLET/include to their list of include directories. Therefore, there will be no errors with this modification.

I have to do nothing of the sort. I was talking about a non-vcpkg-aware project that opportunistically uses vcpkg's WTL includes (and does not use any other vcpkg libraries). I was aware this is a somewhat far-fetched idea, which is why I said so.

I don't think continuing this discussion makes much sense because I cannot convince you that your approach is wrong. Mostly, probably, because I do not understand your reasoning. I'm not saying it does not make sense, it's just that I cannot see that sense at this time.

[dg0yt]

1. wtl is a separate component from ATL, so the compiler will never find them in the include of Visual Studio ATL components.
2. Using "" to include in the same folder is valid. But you can also use <> in relative paths based on include directories.

There is an important difference between #include <A> and #include "A":

• The first form is directly dependent on user-defined search order. This is for headers which come from outside.
• Only the second form ensures that side-by-side files are used first. This is what you normally want, for consistency: installation before user defined search order. And that's why this is the right form for including headers from the same source package.

So IMO #25083 clearly was the right approach: Patch exactly the spots which include headers from the same package, using #include "A".

[ras0219-msft]

After consideration, I think the right fix is actually to put the headers into the top level directory (include/). Our original packaging of the library was incorrect with respect to how upstream expects users to consume it and we should mend that to ensure compatibility with other delivery methods (NuGet, etc).

This may break users of wtl on upgrade, but:

1. The fix is easy (just remove wtl/ from your includes)
2. It's the right thing to do for long-term health.

It's also possible that users won't be broken, because they couldn't have included the headers today anyway due to #24933 without adding the subdirectory.

[chrullrich]

After consideration, I think the right fix is actually to put the headers into the top level directory (include/). Our original packaging of the library was incorrect with respect to how upstream expects users to consume it and we should mend that to ensure compatibility with other delivery methods (NuGet, etc).

I have a philosophical issue with that; I'm not in favor of letting upstream dictate how I'm going to organize myself. Plus, again, the use of <> in atlribbon.h is a bug, and vcpkg has no need to accommodate upstream bugs if it is so very easy to fix them instead as it is here.

1. The fix is easy (just remove wtl/ from your includes)

This probably isn't the first time vcpkg has changed its mind about its internal organization, and won't be the last, but it is certainly in the top league in terms of premeditated unnecessary breakage.

2. It's the right thing to do for long-term health.

How is spamming the include directory with twenty new files good for that?

Also, the include path is a shared namespace, and getting rid of the wtl/ prefix increases the risk of name collisions, if only slightly.

It's also possible that users won't be broken, because they couldn't have included the headers today anyway due to #24933 without adding the subdirectory.

They couldn't have included atlribbon.h anyway, and #24933 is the first time anyone complained about that one. Fixing this single file is trivial and will not break anyone's build, precisely because it could never have worked before. On the other hand, moving all the other headers will inevitably break every project currently consuming any of them via vcpkg.

Can you please wait a bit for upstream to decide whether they want to change the <> to ""? If they do, this whole thing will go away entirely, and entirely peacefully, by updating the port.

[Osyotr]

Plus, again, the use of <> in atlribbon.h is a bug

This is not a bug. Many popular libraries use angle brackets in public headers.

If you use #include <wtl/atlribbon.h>, then your usage is wrong - WTL expects to have it's includes to be in includes paths. You should remove wtl/ prefix and add include/wtl into your include paths or fix WTL port so that installed headers are in include (this is what Robert is suggesting).

[ras0219-msft]

I'm not in favor of letting upstream dictate how I'm going to organize myself.

The objective of vcpkg, excepting specific points that cause integration pain (see our maintainer guide for more details), is to follow upstream as much as possible.

If you would like to privately organize the headers differently than upstream and vcpkg, you can absolutely do that with an overlay port! However, vcpkg's public, curated catalog is intended to model upstream as much as possible.

Can you please wait a bit for upstream to decide whether they want to change the <> to ""? If they do, this whole thing will go away entirely, and entirely peacefully, by updating the port.

Regardless of upstream's decision on that, we would still want to eliminate the wtl/ subdirectory for conformance.