Extend Environment.Dump() to select serialization format

[Xrayez]

Environment.Dump() produces pretty-printable results only, so the usefulness of this method is limited to debugging purposes.

The existing method is extended to allow selecting a serialization format to use via format parameter. Namely, it's now possible to serialize variables as a JSON-formatted string, which makes it possible for automated external tools to inspect the environment more easily.

---

I'm coming from godotengine/godot#37248 and the implementation is preliminary accepted by the @godotengine's project manager and I was advised to create this PR upstream (here). It's not required for the changes to be available in SCons.

Contributor Checklist:

• I have created a new test or updated the unit tests to cover the new/changed functionality.
• I have updated CHANGES.txt (and read the README.rst)
• I have updated the appropriate documentation (though I have to admit that I haven't figured out how to build documentation to test this locally, I'm on Windows and some toolchains are broken there) figured Extend Environment.Dump() to select serialization format #3672 (comment).

[bdbaddog]

can you change to use a env variable to specify the format to Dump()?

env.Dump(DUMP_FORMAT='json')

That way we can add yaml, etc.. if we need to without adding tons of APIs

[Xrayez]

@bdbaddog sure, I've thought that having a dedicated method for this would actually promote explicitness instead. But this would also mean missing out on the ability to actually save the environment directly to a file (would also have to add a filepath parameter), unless you suggest otherwise.

I'll try this tomorrow.

[mwichmann]

I like the idea of extending the API to select format. I'm less fond of Dump's existing API, where Dump(arg) -> <class 'str'> (value only, no key) while Dump() -> <class 'dict'> (keys and values). It's not as bad as Dictionary, where three different arg styles lead to three different return types (Dump calls Dictionary fwiw). Sorry for ranting...

[Xrayez]

I've applied the suggestions and added a dump_format parameter. I removed my initial feature to serialize to file directly, this is likely the thing which must be better handled by the user side as following:

with open('.scons_env_dump.json', 'w') as f:
    f.write(env.Dump(dump_format='json'))

The pretty-printable format is still the default (named pretty), unless you want to change this to json.

I'm not sure about the dump_format name specifically, it's a bit verbose (not really keen to naming this as format either to avoid naming conflicts with string's format...), but nonetheless this works.

[bdbaddog]

I'd rather format than dump_format

[Xrayez]

I'd rather format than dump_format

Done.

---

Also, I've managed to build documentation today to test the updated Environment.xml! I'll even link a screenshot of the HTML man page:

---

I'd also like to give some feedback regarding the steps I've gone through to achieve this as a first time contributor. The SCons Documentation Toolchain suggested me to do so!

This text tries to give an overview of the current SCons documentation
toolchain. As a prospective doc editor, you should be able to quickly
understand the basic concepts (if not, please let the project know how
it falls short) [emphasis mine]. It is also a reference for core developers and the
release team.

As someone who's on Windows (don't get me wrong, I do not discriminate any of the existing OS), I've used WSL to run the following commands:

pip install lxml
sudo apt install fop
python bin/docs-update-generated.py
python scripts/scons.py doc

What made me confused is that running the root script without fop installed prevented me to build the pages at all (I thought it was optional for PDF rendering specifically):

$ python scripts/scons.py doc
scons: Reading SConscript files ...
git    :/usr/bin/git
gzip   :/bin/gzip
unzip  :None
zip    :None
doc: No PDF renderer found (fop|xep)!
doc: ...skipping building User Guide.
     ...creating fake MAN pages.
scons: done reading SConscript files.
scons: Building targets ...
scons: `doc' is up to date.
scons: done building targets.

Also, it would likely be better that python bin/docs-update-generated.py could be run automatically given it only takes a few seconds to update some "esoteric" docs there (and seems like Environment.xml is one of them). 🙂

[bdbaddog]

Yup. The doc tooling could use some work. PR's welcome.. ;)

[mwichmann]

I'd suggest the manpage entry ought to indicate the default, as in:
Returns a pretty printed representation of the environment. (if format is not specified, this is the default)

As to the build, thanks for the feedback... the "generated" docs aren't so esoteric, they are the "shared" data for construction variables, functions, tools and builders - this material is included into both the manpage and into the appendix of the user guide, which is why those items don't appear in either doc/man or doc/user - the duplication needed there would pretty much guarantee they go out of sync immediately. Yes, Environment.xml is one of the source files for that. The tool doesn't run automatically because it's not always needed - if you edit in doc/man or doc/user there's no need to run it. Anyway - that's how it is, not necessarily how it should be.

[Xrayez]

Yeah, I'm just trying to help, I understand that the project has a long history. It's just that a few snippets here and there could already clarify the existing workflow right away. But I also understand that it's a project used by many people and I respect the requirements and dedication needed in order to contribute on the level as big as this project (contributing shortcuts should likely not be encouraged for a project like this).

I'd suggest the manpage entry ought to indicate the default

Done!

[bdbaddog]

@Xrayez - honestly few people try to run the doc flow. So I give you big credit for completing that.. :). There's a wiki page which covers among other things building the generated files for the docs. https://github.com/SCons/scons/wiki/SimplifiedReleaseProcedure

[Xrayez]

Added more robustness:

• raise ValueError if specifying unsupported format (a test added for this expected failure accordingly)
• convert the specified format to lowercase, so if the user passes "JSON", this won't raise the error.

[bdbaddog]

Looks good to me. Merging!
Thanks for the good work!