SCons
Initial VCPkg integration
This change introduces the VCPkg builder integrating the vcpkg system (http://vcpkg.io) for acquiring and building 3rd party C/C++ libraries. This enables an SConstruct file to say, for example, "env.VCPkg('openjpeg')", and then vcpkg will handle downloading the source code and building it. The VCPkg() builder further ensures that the vcpkg include directory gets added to CPPPATH, and that its static lib directory gets added to LIBPATH.
One notable quirk of the VCPkg builder is that it does the actual package build during the initial execution of the SConstruct file, rather than occurring later, during SCons's normal build phase. While undesirable, this ensures that the VCPkg builder is able to accurately compute the set of files produced by building the package (as of this writing, vcpkg does not support any means to predict the files generated by building a package). I hope to improve this in the future.
Note: related issue in vcpkg repo
Contributor Checklist:
- [X] I have created a new test or updated the unit tests to cover the new/changed functionality.
- [X] I have updated
CHANGES.txt(and read theREADME.rst) - [X] I have updated the appropriate documentation
@mwichmann @bdbaddog what are the expectations regarding code coverage? If I'm reading this correctly, my unit tests only exercise 41% of the lines of code. There are certainly more tests that could be written (I've left a TODO in the test file listing scenarios that I think would be reasonable to test), but 100% code coverage is not feasible.
@mwichmann @bdbaddog what are the expectations regarding code coverage? If I'm reading this correctly, my unit tests only exercise 41% of the lines of code. There are certainly more tests that could be written (I've left a TODO in the test file listing scenarios that I think would be reasonable to test), but 100% code coverage is not feasible.
We don't have hard requirements on code coverage. That said if you can reasonable add more tests to cover the major code branches, please do.
Sigh. The doc build failure is one I started to run into as well. I was able to stave it off by backing out an example - that was the thing that triggered it, but there was nothing wrong with my snippets, it just someone was one piece too much for lxml (see #4235). In other words, it's going to be tricky to figure out what's going wrong here and in that PR.
Bummer. Also, on a related note, I noticed that the SconsDoc stuff tries to load lxml, and if that fails, tries to fall back to a different implementation, but that implementation apparently doesn't support XSD schema validation? In any case, it falls down pretty hard, with no clue on how to fix it. I finally figured out I needed to "pip install lxml". IMO, we should rip out the fallback XML parser and instead have a friendly failure message to the person running SconsDoc that they need to install lxml. I'll log an issue on this.
Bummer. Also, on a related note, I noticed that the SconsDoc stuff tries to load lxml, and if that fails, tries to fall back to a different implementation, but that implementation apparently doesn't support XSD schema validation? In any case, it falls down pretty hard, with no clue on how to fix it. I finally figured out I needed to "pip install lxml". IMO, we should rip out the fallback XML parser and instead have a friendly failure message to the person running SconsDoc that they need to install lxml. I'll log an issue on this.
The "new issue" text tells me to raise this on Discord or the mailing list first. Should I log this issue?
Bummer. Also, on a related note, I noticed that the SconsDoc stuff tries to load lxml, and if that fails, tries to fall back to a different implementation, but that implementation apparently doesn't support XSD schema validation? In any case, it falls down pretty hard, with no clue on how to fix it. I finally figured out I needed to "pip install lxml". IMO, we should rip out the fallback XML parser and instead have a friendly failure message to the person running SconsDoc that they need to install lxml. I'll log an issue on this.
The "new issue" text tells me to raise this on Discord or the mailing list first. Should I log this issue?
Please bring it to the users mailing list or the discord server.. :)
So, yes, there was a transitional phase... lxml was the "second choice" in the Python 2 days, but the other approach hasn't been supported for a while, and doesn't wotk on Py3. Not everything relating to "the old days" has been ripped out completely, just because... we're (somewhat) human, I guess. There's a script to set up for development (bin/scons_dev_master.py) which is documented somewhere, and which also has an error in it, so it will blow up after installing the deps - just found that the other day. Meanwhile, in the current source tree (not in any released version) there are now three requirements files: the base one for running scons, requirements-dev.txt which adds the stuff you need basically to run the full test suite, and requirements-pkg.txt, which is "everything" - basically adding the stuff needed to build the docs. lxml is in the requirements-dev.txt since it's used by some of the docbook and xml tests.
Hmmm...so maybe add this to an "else if posix" clause at the end of the "if hpux...else if aix...else if darwin..." structure?
Sent from ProtonMail mobile
-------- Original Message -------- On Sep 18, 2022, 9:19 AM, Mats Wichmann wrote:
@mwichmann commented on this pull request.
@@ -809,6 +809,8 @@ def tool_list(platform, env): 'tar', 'zip', # File builders (text) 'textfile',
# Package management'vcpkg',Ummm...how do I detect "linux"? It seems like tools such as m4 and rpm are being added simply based on "not win32". Is there a positive test I can do to detect a linux system?
Eh, that's a little complex. Depending on where you're asking... a simple way is to check sys.platform for linux. However, the name of the Platform module is actually posix, which is shared with some of the other choices (that originally comes from the value of os.name).
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>
Sigh. The doc build failure is one I started to run into as well. I was able to stave it off by backing out an example - that was the thing that triggered it, but there was nothing wrong with my snippets, it just someone was one piece too much for lxml (see #4235). In other words, it's going to be tricky to figure out what's going wrong here and in that PR.
Hi @mwichmann . I'm finally getting back to this project. Since the doc generation is blocking the PR, I think I might take a stab at fixing that problem. Have you done any investigation on this issue yet? My suspicion is that this might be an out-of-memory issue, either in Python or in FOP (a Java tool, IIRC). I'm wondering if it might be as simple as allocating more "max" heap memory to the JVM that runs FOP. Also, have you been able to get a local repro for this issue? I tried to repro it myself on my Ubuntu WSL virtual machine back in October, but as I recall, I was not able to hit this, at least not on the few attempts I've made.
The doc build part appears to be a built-in limit that turns out to now be tunable thanks to someone's efforts about six months ago. I bumped the limit again for #4263, as those changes pushed me into the "trouble" area, but that has not been merged yet. Whether it triggers does seem to be situational. In any case, if you want, you can try the same bit:
diff --git a/SCons/Tool/docbook/__init__.py b/SCons/Tool/docbook/__init__.py
index 5cf5e6198..52e291144 100644
--- a/SCons/Tool/docbook/__init__.py
+++ b/SCons/Tool/docbook/__init__.py
@@ -69,7 +69,7 @@ re_refname = re.compile(r"<refname>([^<]*)</refname>")
# lxml etree XSLT global max traversal depth
#
-lmxl_xslt_global_max_depth = 3100
+lmxl_xslt_global_max_depth = 3600
if has_lxml and lmxl_xslt_global_max_depth:
def __lxml_xslt_set_global_max_depth(max_depth):
Bummer. Also, on a related note, I noticed that the SconsDoc stuff tries to load lxml, and if that fails, tries to fall back to a different implementation, but that implementation apparently doesn't support XSD schema validation? In any case, it falls down pretty hard, with no clue on how to fix it. I finally figured out I needed to "pip install lxml". IMO, we should rip out the fallback XML parser and instead have a friendly failure message to the person running SconsDoc that they need to install lxml. I'll log an issue on this.
FWIW, there's now a straightforward way to get all the deps in place if you intend to "build scons", which includes building the doc set:
pip install -r requirements-pkg.txt
The doc build part appears to be a built-in limit that turns out to now be tunable thanks to someone's efforts about six months ago. I bumped the limit again for #4263, as those changes pushed me into the "trouble" area, but that has not been merged yet. Whether it triggers does seem to be situational. In any case, if you want, you can try the same bit:
diff --git a/SCons/Tool/docbook/__init__.py b/SCons/Tool/docbook/__init__.py index 5cf5e6198..52e291144 100644 --- a/SCons/Tool/docbook/__init__.py +++ b/SCons/Tool/docbook/__init__.py @@ -69,7 +69,7 @@ re_refname = re.compile(r"<refname>([^<]*)</refname>") # lxml etree XSLT global max traversal depth # -lmxl_xslt_global_max_depth = 3100 +lmxl_xslt_global_max_depth = 3600 if has_lxml and lmxl_xslt_global_max_depth: def __lxml_xslt_set_global_max_depth(max_depth):
OK! I've added this change (I upped it to 4000 for good measure), and that seems to have fixed the issue for me. Thank you!
@jediry - Sorry for the delay. Finally got a big chunk of time to review this and read up on vcpkg and (hopefully) better understand.
Let me ask you a fundamental question about the goal of this functionality. Is it really:
- Check and see if this package is already installed, and if not install it and configure scons to be able to use it's libraries and header files (and binaries)?
- Build this package as part of my build?
I'm thinking it's really #1?
I'm thinking it's really #1?
Correct. Though, if the specified package is not currently installed, its binaries will be built as part of the install process, so #2 kind of applies as well.