algorithm-archivists / algorithm-archive

A collaborative book on algorithms
https://www.algorithm-archive.org
MIT License
2.33k stars 353 forks source link

Using SCons tools #977

Closed Amaras closed 2 years ago

Amaras commented 2 years ago

This is a PR to remove the language builders from the main SConstruct file, and put them into their own tools.

I confirm that everything compiles on my machine, but GH actions will prove me right or wrong on general Ubuntu, and I have no idea what's gonna happen on Windows.

This was inspired by my thought process on #976, and it gives us a structure for future compiled languages.

I did not touch the Copier builder, and there are probably things I could remove for uniformity in rust_SConscript.

Apart from that, if you have any remark, feel free to leave them here.

PeanutbutterWarrior commented 2 years ago

From the SCons docs:

At the moment, user-added tools do not automatically have their exists function called. As a result, it is recommended that the generate function be defensively coded - that is, do not rely on any necessary existence checks already having been performed. This is expected to be a temporary limitation, and the exists function should still be provided.

I'm not sure how this would best be remedied. Immediately returning from the generate functions isn't viable, as the build script sees that as a success and crashes SCons in it's entirety when it tries to build that language with an AttributeError.

I think we should add some sentry value instead of the builder to avoid the AttributeError, and then have the SConscripts check against this value and fail immediately and print some message. I would prefer it fail when SCons tries to build the tool, but I can't find how we would achieve this.

We could have the languages dictionary accessible through the Environment, and remove languages that can't be built from it when the generate function gets called, but I fear this would end up hard to debug and obscure.

PeanutbutterWarrior commented 2 years ago

I'm also not a massive fan of having two directories just for SCons plumbing, but it's just a minor gripe.

Amaras commented 2 years ago

I'm also not a massive fan of having two directories just for SCons plumbing, but it's just a minor gripe.

This could be remedied simply by putting both the builders and sconscripts directories in a scons directory, and changing toolpath=['builders'] to toolpath=['scons/tools'] or something like that

I'm not sure how this would best be remedied. Immediately returning from the generate functions isn't viable, as the build script sees that as a success and crashes SCons in it's entirety when it tries to build that language with an AttributeError.

This is something that is remedied by the Kotlin tool in scons-contrib repository, something like this:


class ToolKotlinWarning(SCons.Warnings.SConsWarning):
    pass

class KotlinNotFound(ToolKotlinWarning):
    pass

SCons.Warnings.enableWarningClass(ToolKotlinWarning)

def _detect(env):
    """ Try to detect the kotlinc binary """
    try:
        return env["kotlinc"]
    except KeyError:
        pass

    kotlin = env.WhereIs("kotlinc")
    if kotlin:
        return kotlin

    raise SCons.Errors.StopError(KotlinNotFound, "Could not detect kotlinc executable")
    return None

with generate calling env['KOTLINC'] = _detect(env). It is probably a good enough solution, but potentially prevents building entirely, so I'll try that.

Amaras commented 2 years ago

Following discussion on Discord, @PeanutbutterWarrior and I converged to this version. The only thing left is to decide whether or not we want to have a single directory for SCons thingies, or if we keep two.