Project module refactor discussion

[StevenClontz]

project.ptx (I volunteer to write a converter from <2)

<project 
    ptx-version="2" 
    source="source"
    publication="publication"
    output="output"
    deploy="deploy"
    xsl="xsl">
    <targets>
        <target
            name="web"
            format="html"
            source="main.ptx"
            publication="publication.ptx"
            output="TARGET_NAME[.ext]"
            deploy="TARGET_NAME"
            xsl="custom.xsl">
            <stringparam key="key" value="value"/>
         </target>
    </targets>
</project>

[StevenClontz]

After today's chat in Zoom, I'm tasked to tackle this + refactoring project.py.

[StevenClontz]

In particular: ingest the XML upfront in order to create a more pythonic object. Also consider separation of concerns between Project and Target.

[bnmnetp]

I would like to make another pitch for two additional attributes on target (optional!)

1. prebuild-hook
2. postbuild-hook

Right now for both Runestone builds as well as working on developing interactive components I have to write additional wrapper scripts around the pretext build command. It would be much faster and more open if we could support those two hooks.

For example a prebuild-hook could run npm build to create a new webpacked set of css and javascript files
A postbuild hook could do the following things that I have to currently write additional scripts for

• copy the webpacked files to the _static folder
• Update the library table in the database
• process the runestone-manifest.xml file
• rsync the final output to worker machines

Those are just my use cases...

[StevenClontz]

If these hooks are generally the same for every book Runestone builds, then I'm inclined to not have authors hardcode them into their project.ptx. That way Runestone doesn't have to request that each author update their hooks for a special Runestone target. But I may be misunderstanding the request.

[StevenClontz]

#464

[StevenClontz]

I've started working on this, but the Codespaces dev environment is stale and preventing me from testing/linting in the UI, so I'm attacking that instead now.

[StevenClontz]

This work may take a chunk out of #461.

[bnmnetp]

I would also like to propose that we include a runestone target by default. There are more and more existing authors that are now wanting to put their books on Runestone. There is no reason for this to be a burden to them when we can provide the basic target or them.

Further we can reduce friction for existing authors by asking for and validating their document-id when they set up a new project either with pretext new or pretext init

[StevenClontz]

One of the things I'm working on is being able to programmatically configure a PreTeXt project within Python, without touching an XML file. What I'd imagine could happen here is Runestone ingests the first html-format Target, then modifies it as appropriate before building. That way the author doesn't need to maintain a separate target for Runestone.

[StevenClontz]

Put another way: ideally the author shouldn't to have to do anything extra to deploy to Runestone compared to deploying to GitHub Pages. Instead, I want to give Runestone the tools to take their source as-is (assuming it builds successfully for web otherwise) and build successfully for a Runestone server.

[bnmnetp]

One of the things I like about the cli is that I have a consistent way to build all of the pretext books
with pretext build runestone This is directly analagous to how I have always built rst books using runestone build --all deploy. What you propose seems like a step backward. To do what you say I would now have to write another script that:

• Reads the project.ptx file and finds and html target (which will hopefully exist) but need not exist it might be html it might be web it might be something else of the authors choosing.
• Reads the publication file for that target and then writes out a new publication file that simply adds <platform host="runestone" /> Then writes that publication file
• Reads in the entire xml book and then query to find the document-id tag within the docinfo tag.
• Assemble the new target and then ???

Put simply, I want to use the cli as a tool in my build chain. I really don't understand the resistance to just adding a target for runestone. Its no more complicated than the latex tags which are included.

publishing on runestone is not the first thing that any author is going to do. So focusing on making it easy for them to do everything but publish on runestone seems reasonable. But why not just add the runestone target when you have asked for all this information during the init or new process rather than making them manually edit something when they are finally ready?

We can easily work together during that init process to try to make the transition to runestone as simple as possible later on.

[StevenClontz]

Set: <stringparam key="debug.rs.services.file" value="file:////your/home/rs/bases/interactives/runestone/dist/webpack_static_imports.xml" /> in the project.pxt file of the book.

Once upon a time this was possible to be passed along the commandline, until I screwed it up and made it be required part of the project.ptx: 0936cb5 (There's a way to use the project-ptx-override option but I'm not thrilled with it either.

pretext-cli/pretext/cli.py

Line 341 in 8fe70f9

@click.option(

)

This is going to be fixed in the refactor: a project.ptx file won't be necessary to instantiate a project if you're running the CLI as a library, or you can modify the properties of a project/target after instantiation.

[StevenClontz]

Likewise, you won't need to run PreTeXt CLI as a subprocess as is done currently in https://github.com/RunestoneInteractive/rs/blob/0355f5dc452d1b5ecacaafd24865dcfc044a26a0/components/rsptx/build_tools/core.py#L152 - instead you can from pretext import Project and use the Project programmatically in your code.

[bnmnetp]

I'm not following you on the stringparam.... It seems to still work for me.

[bnmnetp]

Very nice that we can create a Project and then do stuff to build rather than create another subprocess. Its not a huge win as the build time is so much larger than the cost of spinning off a process.

I do not think that either of your responses directly address my original points. So please elaborate.

[StevenClontz]

I think a proof of concept will be more efficient than elaborating at this moment. Rest assured that I have no plans to push something Runestone cannot use (not that Oscar would let me!).

[StevenClontz]

@oscarlevin I have a WIP that would benefit from your eyeballs if you can sync up with me sometime today.

[StevenClontz]

I'm about to have to step away from this for at least the next week, if @oscarlevin wants to move #464 forward.

I've been fairly vigilant about testing (but not type checking, see #469, though I've been trying to keep things type safe manually), so https://github.com/PreTeXtBook/pretext-cli/blob/refactor-project/tests/test_project.py is a good reference on what's been implemented. Essentially we have builds and asset generation up and running, but not staging/deployment of sites and/or targets. Due to the redesign, it's just as easy to manage your project dynamically in a Python script as it is to configure it via a project.ptx (which ended up being handy as we didn't include project.ptx for some of our templates, so my tests could create the project dynamically instead), and I have legacy support for the current (vague) schema for <2.0 project manifests as well. Note that custom executables are now inferred from an optional executables.ptx that must be a sibling of project.ptx - generally this should be gitignored as it's unique to your machine.

I've tried to touch stuff outside project_refactor.py minimally. But I did move a few things in particular:

• I haven't touched cli.py, but I did adapt the asset caching/generation logic that's there currently into the Target class. I had trouble groking how tuples were used as keys, so the implementation uses {asset_type: {id: hash}} instead, where id=="" means there was no xmlid. (I'm pretty sure you were using {(asset_type, id): hash} which I guess is isomorphic, but I think the API is simpler this way as it avoids having to do stuff like set(tup[0] for tup in keys).)

• I've factored out some constants and custom types into constants.py and types.py. In at least one case these could be synced up better (I think we now have both a format type and BUILD_FORMATS list?)

• I created a wrapper build function within build.py that handles all format types and deals with core library management like core.set_executables. There's probably some DRYing of the other build functions there that could be factored into it if we want.

As of now, all the existing tests still pass, so the changes outside of project_refactor.py cannot be too scary.

Here's what I know needs to happen:

• Implement staging and deployment of sites and build outputs
  • Project().view needs to be revisited for previewing sites.
• Rewire CLI to use project_refactor.py.
  • When doing so, consider if there's CLI functionality that should be Project/Target class functionlity (e.g. asset caching and generation) - we want @bnmnetp and others using pretext as an application dependency to have feature parity with CLI end users.
• Drop the old project.py and rename project_refactor.py to it, rewiring the rest of the library as appropriate.
• Use logging appropriately

[bjones1]

I've pushed some big updates to this. Things to do:

• The project.ptx seem to assume that output defaults to the target name unless output is provided. However, constants.py has output = Path("output". Which is correct? (The first seems better to me.)
• Very sadly, pydantic-xml does not report line numbers on schema violations. I'll submit a feature request for this.
• What should the default for target.publication be? Per the old constants.py, this was Path("publication.ptx"). However, this causes problems in refactor.Target.external_dir() and refactor.Target.generated_dir(), since these both try to open a non-existent file.
• In the new project file format, the <target> tag goes inside the <targets> tag. A suggestion: just omit the <targets> tag, to reduce the hierarchy.
• the build.build function takes a braille_mode parameters. What if this isn't specified in the target, since it's optional?

Tests pass, except for the problems listed above. @StevenClontz or @oscarlevin, can you look at / work on this?

[StevenClontz]

• The project.ptx seem to assume that output defaults to the target name unless output is provided. However, constants.py has output = Path("output". Which is correct? (The first seems better to me.)

project/@output defaults to "output", but project/targets/target/@output defaults to the target's name.

[StevenClontz]

I can see a use-case for output="" distinct from omitting output. I'd expect <project><targets><target name="foo" output=""/><target name="bar"/></targets></project> to build the foo target to output and the bar target to output/bar.

[StevenClontz]

I suppose <target name="foo" output="."/> would be the workaround if there's a good reason to have output="" use the default.

[StevenClontz]

I think if output="" uses the default, we should be committing to have attr="" always be equivalent to omitting attr in the XML. I'm undecided if this is right (with paths we have the . option, but is there a reason some other attr needs to be an empty string?).

[bjones1]

From a pydantic perspective, there's an (ugly) workaround: provide a default of None (which violates the type of Path). This should work fine, but seems a bit ugly/counterintuitive from a code perspective. I can pursue this if you'd like.

[bjones1]

This is done.

[StevenClontz]

• What should the default for target.publication be? Per the old constants.py, this was Path("publication.ptx"). However, this causes problems in refactor.Target.external_dir() and refactor.Target.generated_dir(), since these both try to open a non-existent file.

project/@publication defaults to "publication" and project/targets/target/@publication defaults to "publication.ptx", so both targets of <project><targets><target name="foo"/><target name="bar"/></targets></project> use publication/publication.ptx

[bjones1]

However, this causes exceptions/test failures: refactor.Target.external_dir() raises an exception if publication.ptx doesn't exist. What should we do here?

[StevenClontz]

• In the new project file format, the <target> tag goes inside the <targets> tag. A suggestion: just omit the <targets> tag, to reduce the hierarchy.

What do you think @oscarlevin? We're not using <targets/> for anything, and we're already tightening the schema up (while supporting legacy ver<2 files), so I'm inclined to agree I think.

[oscarlevin]

Where will be put settings related to the landing page? That seems to be a project-level setting. I guess now we have a @deploy attribute on the root project, but what if we want to have settings related to this? Like a no-jekyll option or a different static page generator. What if we want multiple deploy targets (eventually)?

I suppose if we are confident that nothing other than targets will be specified in the project manifest, then we don't need to group them.

Oh, what if we grouped targets by deployment? Like one <targets name="deploy"> and another <targets name="private"> or <targets name="development">?

[StevenClontz]

Eliminating <targets\> does not prevent <project\> from having other children like a <deploy\>. Being able to group a bunch of targets and have common configuration seems reasonable as well though.

[StevenClontz]

the build.build function takes a braille_mode parameters. What if this isn't specified in the target, since it's optional?

@oscarlevin knows braille stuff much better. N.B. xml_id is no longer allowed in project.ptx as it's passed as an argument (e.g. an option within the CLI), since subset builds are not meant for production. Is braille_mode similar or is there a reason someone might want a target with braille_mode set to one thing vs. another?

[oscarlevin]

Two things.

1. I don't know braille stuff well enough to know whether we need to let a target specify the mode, but I would guess we should allow this.
2. Why is xml_id going away? Does the project manifest only get used for production? Especially for use with pretext-tools, it is nice to have a target where you have specified the subset, so you can attach this to a shortcut.

[StevenClontz]

Re: ptx-tools, I'd much prefer a way to tell the extension the xmlidroot I want to build. One problem with subset in project.ptx is that its tracked by Git: do we really want to track when an author changed their preference for what subset to build? Is there a use case to hard code a specific subset?

[oscarlevin]

Why wouldn't we want to track that? It would be helpful for me to know, when I move to my home computer, what subset I was working on in the office. It makes sense to sync this setting.

How would pretext-tools specify the xml:id? After hitting the pretext button, selecting build, selecting the target... do we always then prompt for an xml:id? Only for some targets? Okay, maybe a new function (build subset) that after selecting the target prompts for the xml:id, and remembers the last id that you used that session..., but this is extra button presses. I suppose we could parse the text and offer a list of possible xml:id's, which sounds like a good feature, but does slow down building.

[bjones1]

The CodeChat GUI does this automatically, by passing the first xml:id it finds in the file.

[bjones1]

I've fixed what I can -- waiting on input now.

[oscarlevin]

I must admit I've lost track of the plan for this, so @StevenClontz, can you take stock of where we are and what we need to do to get this folded in? I have time to help finish this off now, but not sure what needs to get done still.

[StevenClontz]

I think syncing up with you and @bjones1 would be useful to advance this. I'm in a virtual conference this week but should be able to join today's dropin at 3:30 Central if you two will be there.

[bjones1]

Sounds good! I'll need to leave around 4:00 PM Central, but I think we can make some good progress!

[oscarlevin]

It looks like I can be at dropin for the first 30 minutes and hopefully at 3:30 CDT to further sync up.

[bjones1]

I've made some good progress, but need help at this point. The lastest pushes get almost all tests passing, except a failure involving WebWork conversion, where fails to find the resulting generated WebWork output. I'm not sure how to debug this.

[StevenClontz]

I'm working on a PTX book this morning. I just logged into prose.runestone.academy/dropin if you want to sync up.

[bjones1]

@oscarlevin, can you help with this? @StevenClontz and I chatted, and we dubbed you the expert :). What we see: on line 205 of the CI output, it looks like spaces are translated to %20 in file paths:

    * failed to load external entity "file:/C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/
pytest-0/test_build_subprocess_0/test%20path%20with%20spaces/generated-assets/webwork/webwork-representations.xml"

Any idea why/how?

[oscarlevin]

I'm on it.

[bjones1]

Fantastic, thanks! I messaged you on Discord -- if you'd like to chat there or via Zoom/whatever, let me know. I'd be happy to show you where I'm stuck if that helps.

[oscarlevin]

Okay, I think I know what the issue is with the test not passing: the other generate steps assume that there is a webwork representation file in generated-assets and tries to import that when generating their assets. Thus, if you run pretext build and you have multiple assets to generate, it will fail the first time, since there isn't yet a webwork-representations file to reference, but will succeed the second time, since the first pass generated it.

This is a fairly major bug with our 1.7 release, so I'll fix this asap, release a bug fix, and we can merge that into the refactor-project branch.

[bjones1]

Wow, way to go Oscar! That's a tricky bug. Nice!

[StevenClontz]

FYI here's where, once the refactored project is available in nightlies, I plan to try out the dynamic target editing to run some custom XSL to produce individual slideshows for each section of the TBIL Linear Algebra text:

https://github.com/TeamBasedInquiryLearning/linear-algebra/blob/2023-slides/slides/build.py

[bjones1]

Neat!

[oscarlevin]

Okay, the webwork-representations.ptx issue is fixed. But now we are stuck on the pickle server error. I really don't know what to do with that.

[StevenClontz]

One error had to do with a bug in Path().resolve() in Windows which I think I've fixed now (waiting on test).

The other seems related to https://stackoverflow.com/questions/52265120/python-multiprocessing-pool-attributeerror

[StevenClontz]

I have a meeting in 9 minutes - let's get this into main so I can start user testing with linear.tbil.org (I'm going to push a bugfix release so I don't have to wait for tomorrow).

[StevenClontz]

@bjones1 can we move the mypy part of the workflow before installing e.g. xelatex? That should fail quicker when types are the issue in that case I think.

[bjones1]

@StevenClontz, I messaged on discord -- I'm happy to work on mypy fixes, but won't want to overlap with your edits...

[StevenClontz]

I'm done here, once the tests pass I'm merging to main, so work on type stuff in a separate branch/PR

[bjones1]

Will do.

[bjones1]

@bjones1 can we move the mypy part of the workflow before installing e.g. xelatex? That should fail quicker when types are the issue in that case I think.

Sure!

[StevenClontz]

You can also address #464 (comment) while you're mucking around the workflow stuff.