deplinenoise / tundra

Tundra is a code build system that tries to be accurate and fast for incremental builds
MIT License
438 stars 75 forks source link

Tundra prematurely stops looking for "Depends" #340

Closed leidegre closed 1 year ago

leidegre commented 1 year ago

I have a unit definition like below where there's a nil value in Depends which results in a linker error. If I move the package_deps[libname] last in the dependency list it works. I don't know if this is intentional or not but it caused an issue for me since I have this "convention" based approach to generate units based how I've organized stuff on disk. I got a linker error because sometimes the package_deps table doesn't have any deps for libname.

I can work around this by moving it to the end of the Depends table/list or by change the line to package_deps[libname] or {}, either is fine.

Program {
    Name = testname,
    Depends = {
        package_deps[libname], -- must be last because if nil tundra will stop looking for more deps...  
        libname, -- implicit
        "libtest",
        "win32", -- Windows specific???
    },
    Sources = {
        test[2]
    }
}

If this is not intentional I think Tundra should either crash with appropriate error or keep looking for dependencies in the list even if some of them are nil. Crashing (or at the very least emit a warning) might be appropriate because it could be a mistake if you have a local that is holding on to some dep information.

I'll happily commit to doing some work here If I can get some direction and a place to start.

leidegre commented 1 year ago

I put together a minimal repo here.

deplinenoise commented 1 year ago

This is an issue with Lua "arrays" in general, and ipairs specifically. For example:

a = { nil, 1, 2, 3 }
for _, val in ipairs(a) do
  print(val)
end

Will not print anything. The reason is that ipairs probes the table for successive keys but stops when it finds nil.

To work around your case, you have a few options:

deplinenoise commented 1 year ago

Marking will not fix, as this is a Lua issue.

deplinenoise commented 1 year ago

That said, if you do want to attempt a fix, you could look here:

https://stackoverflow.com/questions/40441508/how-to-represent-nil-in-a-table

We'd have to rewrite the entire array/list utility library to not use ipairs directly, but instead handwrite loops and/or represent nil some other way, which is pretty awkward.

leidegre commented 1 year ago

Okay, if this is deeply engrained in how Lua works then it's just a quirk. I didn't know Lua operated this way. It sounds like it's just going to add overhead for a very small win. There are many ways to workaround this issue and I'm fine with that.

Though from a UX perspective I think it could be a good thing to generate a warning if it is possible to cheaply detect this. Didn't take me that long to track down but it was confusing when I got linker errors for stuff that shouldn't...

leidegre commented 1 year ago

Maybe something like this?

tbl = {1,2,3,nil,5,6,7}

n = 1
for k,v in ipairs(tbl) do
  print(k, v)
  n = n + 1
end

print(#tbl, n)

if n < #tbl then
  print("warning: incomplete depends. unit xyz might be missing dependencies due to a nil value in the depends table")
end

This is probably an oversimplification, it's not going to be this easy but in principle, this should work.

deplinenoise commented 1 year ago

Yeah. Maybe worth putting something like this in utils and calling it for input arrays, discarding nil as appropriate?