tl: add build options

[euclidianAce]

Adds luafilesystem dependency and the following config options

• build_dir
• source_dir
• files
• include
• exclude
• Original include -> include_dir

Hopefully these changes should remove the need for a Makefile for pure Teal projects (though probably not tl itself)

This contains some pretty big refactors to tl that I will try to outline here.

• Setting up the environment and preloaded modules has been abstracted into some methods for reusability
• tl gen and tl check now build up a source map for file names (src_map: {string:string})
  • tl run will not build up a source map because it only relies on the args passed to it.
  • In the future there could probably be a source_map config option as well to just directly map inputs and outputs.
• There is some custom logic to recursively traverse sub-directories and build up the source map, This could probably be optimized in the future.

Some notes:

• Technically the lfs dependency can be made optional and only required to use things like source_dir. Technically build_dir can get by with some os.execute("mkdir ...") stuff, but that always feels hacky.
• This currently passes the test suite but I'm am unsure of how to write tests for these features, so help on that would be appreciated.
  • Especially with the **/ pattern matching functions, there doesn't seem to be a way to extract local functions with the debug library, so putting them in a module seems like the only way to test them, but having a test suite for these would be helpful.

Constructive criticism/reviews are definitely appreciated, especially for testing this. 🎉

[euclidianAce]

(Should close #47, #31, and maybe #107?)

[hishamhm]

This is great! I left some minor reviews from a quick reading of the PR but haven't gotten around to running it yet

[hishamhm]

@pdesaulniers Could you take a look at this PR? :)

[pdesaulniers]

OK, I'm looking at this.

[pdesaulniers]

Especially with the **/ pattern matching functions, there doesn't seem to be a way to extract local functions with the debug library, so putting them in a module seems like the only way to test them, but having a test suite for these would be helpful.

So far, the CLI options are tested by calling tl from the shell and looking at its output, like this:

tl/spec/cli/include_spec.lua

Lines 4 to 16 in 5732126

	it("adds a directory to package.path", function()
	local name = util.write_tmp_file(finally, "foo.tl", [[
	require("add")
	local x: number = add(1, 2)
	assert(x == 3)
	]])
	local pd = io.popen("./tl -I spec check " .. name, "r")
	local output = pd:read("*a")
	util.assert_popen_close(true, "exit", 0, pd:close())
	assert.match("0 errors detected", output, 1, true)
	end)

Right now, it would be a bit difficult to write this kind of tests for include and exclude, because these options have to be set through tlconfig.lua, and tl assumes that tlconfig.lua is contained inside the current working directory.

So, for now, I would suggest to add some command line options for include and exclude, and use these in the tests.

Otherwise, we could also use a new -p --project option for setting the path of tlconfig.lua, but this sounds a bit harder to manage (we might need one tlconfig.lua per test)

[pdesaulniers]

(Should close #47, #31, and maybe #107?)

Yes for the first two, but I'm not sure about #107. I think this issue proposes to merge all of the command line and tlconfig.lua stuff inside a single Lua table. That way, I suppose we could get rid of the valid_keys table, generate the list of compiler options programmatically, etc.

[euclidianAce]

tl assumes that tlconfig.lua is contained inside the current working directory.

Since we're bringing in lfs for this feature, we could probably use lfs.chdir and build up some directories in /tmp (maybe copying the tl executable too?) and just throw some helloworld.tls and the tlconfig.luas that we want to test in there. When I get the chance I'll try to write something up.

[hishamhm]

@euclidianAce sounds like a plan!

[pdesaulniers]

Minor nitpick: ideally, the --include flag should be renamed to --include-dir here

tl/spec/cli/include_dir_spec.lua

Line 3 in d3236c9

describe("-I --include argument", function()

[euclidianAce]

I've made some pretty big refactors to this now that I've been able to test it and I want to just outline them here:

• Previously tl gen was modified to build a project when run with no arguments. This has been reverted back and I've added tl build which has that behavior
• tl build has a new flag associated with it (name inspired from Gentoo's Portage)
  • -p --pretend: don't compile, but type check and print what files would be written to
    • Similarly from Gentoo, an --ask flag could easily be implemented as well
• The current tl gen now works more like gcc:
  • files are generated in the current directory unless specified with -o --output
  • multiple files are concatenated into one when using -o
    • This feature practically comes for free, by just appending instead of writing. With how the current compiler works, source maps may not be all that hard to implement since there is no minifying. Though in the future it would probably be smarter to concatenate the file contents, then compile.
    • Idk it that's enough to close Add some way to bundle multiple .tl scripts as one Lua file? #67 but its a start.
• --skip-compat53 can be put in tlconfig.lua as skip_compat53 = true, unrelated, but I'm using this pr with some of my own projects and it was annoying to type each time.

Internally, the code is a lot cleaner and partitioned for each command. With the forcing of relative paths the pattern matching and source file mapping has become a lot easier to manage as it was previously a hodgepodge of to_relative calls. There is also a project object that gets built up with the build option, and this seems like a good candidate for something to expose as an api as it has methods attached to it for traversing and filtering the project. And tlconfig could easily be attached to it.

The test coverage is pretty alright, but I still need to write more :P

[hishamhm]

@euclidianAce wow, impressive!!

Some commentary based on the above comment (haven't looked deeply into details of the code yet)

Previously tl gen was modified to build a project when run with no arguments. This has been reverted back and I've added tl build which has that behavior

Nice!

-p --pretend: don't compile, but type check and print what files would be written to

I've seen this being called --dry-run in other programs, but I guess --pretend works just as well!

multiple files are concatenated into one when using -o

This bit I am a little concerned with. If multiple files just declare global functions, then this will work fine, but if each file declares a module, then mere concatenation of the outputs will not produce what I would consider the expected behavior: amalgamation scripts generally end up declaring multiple independent modules (like one would see on a big .so C module that declares multiple Lua modules). I would rather drop this feature for tl gen for now (producing an amalgamation script could be done as a separate feature in a separate PR — let's restrict PR scope to get this one merged sooner :) )

--skip-compat53 can be put in tlconfig.lua as skip_compat53 = true, unrelated, but I'm using this pr with some of my own projects and it was annoying to type each time.

👍

The test coverage is pretty alright, but I still need to write more :P

If the coverage is alright I guess we can look into reviewing and merging this version. Let's just make sure the new features are covered in the docs.

Very cool work in this PR, improving the tooling is super important!

[euclidianAce]

For the time being I'll limit --output to asserting only one script argument. Didn't really think of the consequences of that one.

But this is pretty feature-complete for what I imagined the initial write up of this would be and while I still have some ideas, I think it would be feature creep. So over the next few days I'll try to improve test coverage and documentation to get this ready to merge. 🎉

[euclidianAce]

I'll leave this here as a reference for where I think I'm at and suggestions for where to go.

TODO

• Test:
  • source_dir
  • build_dir
  • any file including option with relative vs non-relative paths
  • include+exclude globbing
  • basic interactions between include+exclude
  • include+exclude with multiple patterns
  • basic interactions between source_dir+build_dir
  • basic interactions between source_dir+build_dir+include+exclude
  • error handling for source_dir
    • i.e. source_dir not found: error out gracefully, probably mentioning source dir in some way
  • error handling for build_dir
    • there is some error handling, but not extensively tested yet
  • -o --output
  • -p --pretend --dry-run
    • honestly this could just be a copy-paste of any instance where util.run_mock_project was used and just add -p and verify the output
  • tl build
    • any usage of util.run_mock_project is basically the test suite for build, so I'm pretty confident in the current behavior of it.
    • but there could be more explicit tests
• Document:
  • -s --source_dir
  • -b --build_dir
  • files tlconfig.lua entry
  • include+exclude
    • globbing patterns
  • -o --output
  • -p --pretend --dry-run
  • tl build
  • skip_compat53 tlconfig.lua entry

In general, I think I need to write more tests where the expected output is an error.

[hishamhm]

I'd like to get this merged as soon as possible since it's getting too big to do a review in one go. :)

I took another look at the PR (not a super detailed line-by-line look, I admit! it's a lot of stuff!) and left a couple of comments, apart from those I think we can get it merged now and if anything needs tweaking we can fix in future commits/PRs

[euclidianAce]

I'd say go ahead and merge this if you'd like. The most basic error cases have tests now and I've been using this since I made the pr on a bunch of tiny things and it hasn't had any trouble.

Here's a rundown for the changelog:

• tl gen now generates the file in the current directory, much like gcc and other tools
  • -o --output now lets you specify the output for a single script
• tl build is a new command that will type check and compile a given set of files specified in tlconfig.lua of the current working directory or the command line. These new entries/flags are:
  • -b --build-dir build_dir: string: where generated files will end up
  • -s --source-dir source_dir: string: when provided, build will only include files from inside the given directory
  • files: {string}: a list of filenames to compile
  • include has been renamed to --include-dir include_dir: {string} to account for:
  • include, exclude: {string}: a list of patterns to find files to compile, what patterns are available are outlined in docs/compiler_options.md
• New flag -p --pretend --dry-run: don't write to any files, but type check and log what files would be written to.
• New config entry skip_compat53: boolean same as --skip-compat53

[euclidianAce]

Hey @hishamhm, any updates on getting this merged?

[hishamhm]

@euclidianAce hi, sorry for dropping the ball on this! I've been unable to put too much energy on Teal the last couple of weeks. I just merged it manually, squashing some of the commits together. Thank you for the nudge, this PR is a huge win for the project!

[euclidianAce]

Thanks, no worries!