Time  Nick       Message
01:42 HuguesRoss Can someone clarify the relationship of a couple PRs for me? I'm looking at #10486 and #11438, both aim (at least in part) to fix issue #8601. However, the latter of the two has an approval and is assigned to a milestone while the former hasn't had any activity in ~6 months but seems to be waiting on review...should we consider closing it, or has it been left open for a reason?
01:42 ShadowBot  https://github.com/minetest/minetest/issues/10486 -- Fix optional dependencies causing cascading failures with dependent mods by IceDragon200
01:42 ShadowBot  https://github.com/minetest/minetest/issues/11438 -- Mods: Improve mod loading priorities (2nd) by SmallJoker
01:42 ShadowBot  https://github.com/minetest/minetest/issues/8601 -- Optional dependencies being overridden/turned into hard dependencies
01:46 MTDiscord  <Jonathon> the latter before it was closed and reopened a few? times said it superseeded the other
01:46 HuguesRoss Yes, that's what it looks like
01:47 HuguesRoss Today, it's open with a milestone and one approval, so my initial instinct is that it'll probably be the one that gets in
01:49 MTDiscord  <Jonathon> maybe its like the fonts, both are left open till one wins (gets merged)
13:36 MTDiscord  <luatic> HuguesRoss: be careful
13:36 MTDiscord  <luatic> this has the potential to cause massive breakage
13:36 MTDiscord  <luatic> and I fear Krock's implementation might be incorrect
13:38 sfan5      that's why it's in the milestone for 5.6 ;)
13:39 MTDiscord  <luatic> I'll try to sum up the issue
13:39 MTDiscord  <luatic> Because the way I see it, we're still lacking expected behavior for the handling of "optional" dependencies
13:41 MTDiscord  <luatic> The docs say the following: "Like a dependency, but no error if the mod doesn't exist."
13:41 MTDiscord  <luatic> So what should happen if there are circular optional dependencies among enabled mods?
13:42 MTDiscord  <luatic> Simplest (and most understandable) way would be to throw an error, effectively turning "optional" dependencies into "hard" dependencies if the mod exists.
13:43 MTDiscord  <luatic> Alternative is trying to maximize the number of fulfilled optional dependencies
13:44 MTDiscord  <luatic> This might lead to "weird" (unexpected) behavior that a mod is an enabled, optional dependency, yet loads after it's dependant.
13:46 MTDiscord  <luatic> This is effectively a topological sorting
13:48 MTDiscord  <luatic> See modlib for an example Lua implementation: https://github.com/appgurueu/modlib/blob/master/minetest/misc.lua#L242-L323
13:49 sfan5      'turning "optional" dependencies into "hard" dependencies if the mod exists' <- wasn't this the exact behaviour that was considered bad?
13:55 erlehmann  the toposort thing seems true from my POV
13:55 erlehmann  i agree with luatic that the semantics of optional dependencies are at least a bit weird
13:58 sfan5      how do other package managers solve this?
13:59 MTDiscord  <luatic> sfan5: I believe package managers don't bother usually, as they just install the required deps (often you must install "recommended" opt. deps yourself)
14:00 sfan5      hm yeah I guess this exact problem does not apply to them
14:03 MTDiscord  <luatic> As said, we must define how we want optional dependencies to work
14:03 MTDiscord  <luatic> It's odd if optional dependencies don't get fulfilled arbitrarily in circular cases
14:04 MTDiscord  <luatic> I don't like Krock's PR as it isn't optimal
14:05 MTDiscord  <luatic> A recursive solution like #10486 is the way to go
14:05 ShadowBot  https://github.com/minetest/minetest/issues/10486 -- Fix optional dependencies causing cascading failures with dependent mods by IceDragon200
14:05 erlehmann  sfan5 package managers have required and different levels of optional: suggests, enhances, recommends
14:06 erlehmann  sfan5 i believe only required is a strict dependency in terms of topological sort. everything else does not care about loading order. for example, ghostscript does not have to be installed before gimp, as it is merely recommended.
14:07 MTDiscord  <luatic> MT's optional dependencies often very much care about loading order
14:07 erlehmann  i agree with luatic that a recursive solution is probably the way to go. i have seen a lot of ”1. toposort 2. load” approaches turn out badly.
14:33 rubenwardy <sfan5> 'turning "optional" dependencies into "hard" dependencies if the mod exists' <- wasn't this the exact behaviour that was considered bad?
14:33 rubenwardy The problem was because the optional dependencies was then disabled, I believe
14:34 rubenwardy I'm in favour of making failing to load rather than disabling mods when there are missing dependencies
14:35 rubenwardy if an optional dependency exists, then it should be considered a hard dependency from that point on. So if you get a dependency cycle, fail to load rather than randomly disabling mods
14:35 MTDiscord  <Warr1024> Silent failures are bad in general, an starting up a world with requested mods not actually loaded would be a case of that.
14:35 rubenwardy yeah. It's confusing to users
14:38 rubenwardy <rubenwardy> The problem was because the optional dependencies was then disabled, I believe
14:38 rubenwardy more accurately, the problem is with mods being disabled in a chain. If you error instead, it becomes a lot more usable
14:40 MTDiscord  <luatic> That's exactly what I wanted to hear
14:40 MTDiscord  <Warr1024> If I tell it to load a mod and that mod fails to load, then everything should just abort at that stage.  It's not even relevant how mods should respond to their dependencies being present but not loaded because failing to load exactly the set of mods I demanded is already a fatal failure case.
14:41 MTDiscord  <luatic> 100% agreed, I even filed a bug as the errors sometimes don't get logged to chat
14:41 rubenwardy I had a security issue on CTF because of this
14:41 rubenwardy the dependency system silently disabled the mod that adds moderation features, such as locking chat commands behind a threshold of activity
14:42 MTDiscord  <Warr1024> It makes upgrades a nightmare too, because any time you change anything about your setup, mod version or engine version, you run the risk of one brick disappearing and taking out the whole wall without you even realizing.
14:42 rubenwardy I'd like to refactor the mod/pkg system to use a single code base, rather than being split into C++ and Lua. Then I'd like to update the Select Mods to call the dependency resolver and highlight any mods that are missing deps or have cycles
14:42 MTDiscord  <Warr1024> Basically having any mod fail to load is already a fatal error, it's just that we have a human-in-the-loop process for actually detecting it.
15:40 nore       Just giving my $0.02 on the issue: for me, the "intuitive" semantics of "A soft-depends on B" is: "if A and B are both enabled (not only exist), then B is loaded before A"
15:41 MTDiscord  <Warr1024> I had assumed that "exist" in this case meant "exist within the set of mods allowed to be loaded in this world"
15:41 nore       I need to check minetest.get_modpath, but I expect minetest.get_modpath to return nil if the mod provided as argument is present but not enabled
15:42 nore       yep, same
15:44 nore       For me, all functions that check if some mods exist or this kind of things should behave identically if the mod is in the mod folder but not enabled or if the mod is not in the mod folder
15:47 nore       (also, I can write a topological ordering with linear complexity if needed)
15:49 MTDiscord  <luatic> nore: We're considering pathological cases here, so the question is: What if A and B soft-depend on each other (cycle) and are both enabled? Should it throw an error or just silently "violate" one of the two optional dependencies?
15:50 MTDiscord  <luatic> I prefer the error
15:51 MTDiscord  <Benrob0329> Optional Deps are still deps, they need to make sense or there's no right way to resolve them.
15:51 MTDiscord  <luatic> Yeah, but the word "optional" insinuates that they needn't be resolved
15:51 erlehmann  i believe the first step is to clarify what “optional dependency” *means*
15:51 nore       according to the semantics I gave above, an error should occur
15:51 MTDiscord  <luatic> erlehmann: absolutely
15:51 MTDiscord  <luatic> the docs are too vague currently
15:52 erlehmann  i *think* that it indeed involves “if this exists, load it before the current thing”
15:52 MTDiscord  <luatic> https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L209
15:52 nore       that's why I wanted to give a precise semantics for it
15:52 MTDiscord  <luatic> "Like a dependency, but no error if the mod doesn't exist."
15:52 MTDiscord  <luatic> So according to this, it becomes a hard dependency if the mod "exists" (is enabled).
15:52 erlehmann  luatic which is not precise enough, as you pointed out yeah
15:52 erlehmann  debatable
15:52 MTDiscord  <Benrob0329> Luatic: that seems sane and expected to me
15:52 erlehmann  because “no error” does not fit with circular deps
15:53 nore       the "no error" is only if the mod doesn't exist
15:53 MTDiscord  <Benrob0329> Doesn't exist is not the same as exists and is broken
15:53 erlehmann  ah sorry
15:53 MTDiscord  <luatic> it is "no error if the mod doesn't exist"
15:53 erlehmann  nore luatic i should take a nap sorry.
15:53 MTDiscord  <luatic> the only issue I see with the current wording is the "exists"
15:54 MTDiscord  <luatic> it implies that even disabled mods turn soft dependencies into hard dependencies
15:54 MTDiscord  <luatic> WAIT
15:54 nore       yeah, the semantics of exists should be clarified to mean that the mod should be enabled
15:54 MTDiscord  <luatic> according to this, the issue is not a bug
15:54 MTDiscord  <luatic> #8601 is not a bug LMAO
15:54 ShadowBot  https://github.com/minetest/minetest/issues/8601 -- Optional dependencies being overridden/turned into hard dependencies
15:55 erlehmann  if minetest is made into a movie, it needs to be directed by m. night shyamalamadingdong
15:55 erlehmann  twists upon twists
15:56 nore       https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L4569
15:56 MTDiscord  <luatic> https://github.com/minetest/minetest/issues/8601#issuecomment-1011190822
15:56 nore       So minetest.get_modpath is well-documented
15:56 MTDiscord  <luatic> Apparently "exists = installed" as expect
15:57 MTDiscord  <luatic> Funnily enough this forces you to enable optional dependencies, doesn't it?
15:57 nore       I'd still say it is a bug, exists = installed means installing a new mod without enabling it can prevent worlds for loading
15:57 MTDiscord  <luatic> Actually the bug might be that mods still load despite some of their optional dependencies being disabled
15:58 nore       *from
15:58 MTDiscord  <luatic> The question is: What do we consider expected behavior?
15:58 MTDiscord  <luatic> The docs are rather clear (yet silly) on this
15:59 MTDiscord  <GoodClover> I interpret optional_deoendency as "load after if enabled".
16:00 MTDiscord  <GoodClover> Loading an optional dependancy after the dependent mod would be stupid and likely to break things.
16:11 erlehmann  i have a salomonic solution
16:11 erlehmann  you have the entire cdb
16:11 erlehmann  you have the dependency tree of the entire cdb
16:11 MTDiscord  <luatic> hehe
16:11 MTDiscord  <luatic> I like this
16:11 MTDiscord  <luatic> will do
16:11 erlehmann  surely rubenwardy can give us access to the cdb thing
16:11 MTDiscord  <luatic> we don't need access
16:11 erlehmann  and then we can empirically figure out what people actually think
16:11 MTDiscord  <luatic> the API suffices
16:11 erlehmann  ah well but then you have to scrape it?
16:12 erlehmann  but yeah
16:12 MTDiscord  <luatic> I don't have to scrape it
16:12 MTDiscord  <luatic> The JSON API fully returns depends
16:12 erlehmann  i think the solution is to try to figure out how people emprically use stuff
16:12 MTDiscord  <luatic> I don't use it any more than any normal MT client I believe
16:12 erlehmann  luatic the thing is, you have to do it for *all* packages
16:12 erlehmann  this is kind of a big task
16:12 erlehmann  figure out which circular optional dependencies exist
16:12 MTDiscord  <luatic> it's not
16:13 erlehmann  if none exist
16:13 MTDiscord  <luatic> only thousands of mod
16:13 erlehmann  we are lucky
16:13 MTDiscord  <luatic> finding a cycle is linear time
16:13 erlehmann  i mean it is a big task compared to just defining something arbitrarily
16:13 erlehmann  but i think it is the best way forward for the discussion
16:13 MTDiscord  <luatic> aww too bad, https://content.minetest.net/api/packages/?type=mod doesn't contain the depends
16:13 MTDiscord  <luatic> erlehmann: I'm wondering now
16:14 erlehmann  wondering what
16:14 MTDiscord  <luatic> should I check only among mods?
16:14 MTDiscord  <luatic> Or mods + games?
16:14 MTDiscord  <luatic> games + games kinda makes no sense...
16:14 erlehmann  i would check among games first. because they constitute mods that are definitely used together.
16:15 erlehmann  and then try to find cycles in the rest
16:17 MTDiscord  <luatic> I wouldn't check inside games
16:17 MTDiscord  <luatic> That's hardly possible without.. actual scraping
16:17 MTDiscord  <luatic> Just among different CDB packages
16:18 MTDiscord  <luatic> Basically, what if you enabled all mods - would there be an optional dependency cycle?
16:18 MTDiscord  <Warr1024> Some games do have optional depends on other stuff, so they can be involved in cycles, at least...
16:37 MTDiscord  <luatic> https://gist.github.com/appgurueu/31b5b7a6ac5ae5f34b4b3054341763fd - Lua PoC for a mod load order implementation, test it out
16:50 erlehmann  luatic i hereby want to warn you: if you actually enable about a thousand mods, minetest takes very long to start
16:50 erlehmann  even if those are just empty
16:50 erlehmann  and contain nothing but meta info
16:51 erlehmann  okay and an empty init.lua bc otherwise nothing works
16:51 erlehmann  > -- Determines the load order by finding roots for the mod forest and then doing a DFS
16:51 erlehmann  poetic!
16:52 MTDiscord  <luatic> erlehmann: don't worry, I'll just simulate it
16:52 MTDiscord  <luatic> The current algo is most likely trash
17:05 Pexin      out of order optional loads cannot be an option because mods are not like linux packages for example in that elements of mods can be specifically overridden by later loading mods
17:09 rubenwardy luatic, erlehmann: https://content.minetest.net/api/dependencies/
17:10 MTDiscord  <luatic> rubenwardy: thanks, found that already
17:10 rubenwardy I added it just now
17:10 MTDiscord  <luatic> this is my script so far
17:10 MTDiscord  <luatic> https://cdn.discordapp.com/attachments/747163566800633906/930871419368263690/scan-cdb.lua
17:10 MTDiscord  <luatic> NVM msiread
17:10 erlehmann  neat rubenwardy
17:11 MTDiscord  <luatic> thought you meant the per-package dependency API
17:11 rubenwardy this one returns for all packages, which will be more efficient for that usecase
17:11 MTDiscord  <luatic> I guess this will speed things up ;)
17:11 rubenwardy it supports filtering as well
17:11 rubenwardy https://content.minetest.net/api/dependencies/?type=game&type=mod
17:12 MTDiscord  <luatic> yay
17:12 rubenwardy maybe it should filter out txps by default, as they can't have deps
17:12 MTDiscord  <luatic> argh this is tricky because of forks
17:13 rubenwardy yeah, CDB also won't tell you which specific mod in a package depends on what
17:21 MTDiscord  <luatic> looks like there's a bug in my PoC
17:24 MTDiscord  <luatic> bruh ruben, why the pagination
17:24 MTDiscord  <luatic> is there are a way to set per_page?
17:28 MTDiscord  <luatic> erlehmann, rubenwardy: looks like we've got our first circular dependency "moreblocks<-moretrees<-flowerpot<-frame<-moreores<-basic_materials<-moreblocks"
17:28 MTDiscord  <Warr1024> pagination is often done to limit the ability of a single request to impact server resources
17:29 rubenwardy takes 15 seconds to load unpaginate
17:29 rubenwardy +d
17:29 MTDiscord  <luatic> ah alright
17:39 Krock      will merge #11942 in 10 minutes
17:39 ShadowBot  https://github.com/minetest/minetest/issues/11942 -- Fix NodeDef backwards compatibility to 5.3.0 by SmallJoker
17:43 MTDiscord  <luatic> Alright, so here are the cycles my script found among only mods:
17:44 MTDiscord  <luatic> moreblocks<-moretrees<-mg<-moreores<-basic_materials<-moreblocks<-ethereal<-techpack
17:44 MTDiscord  <luatic> technic<-appliances<-clothing<-skinsdb<-i3<-technic legendary_armor<-legendary_ore<-legendary_armor
17:46 MTDiscord  <Warr1024> that last one is legendary_facepalm...
17:46 MTDiscord  <Warr1024> obviously-same-author should know better than to create a trivial loop within their own stuff
17:47 rubenwardy are you ignoring modpacks there?
17:48 MTDiscord  <luatic> Warr1024: yeah, the two mods have each other as optional dependencies
17:48 MTDiscord  <luatic> rubenwardy: ?type=mod
17:48 MTDiscord  <luatic> here it is
17:48 MTDiscord  <luatic> https://cdn.discordapp.com/attachments/747163566800633906/930880968812822579/scan-cdb.lua
17:48 Krock      merging
17:49 rubenwardy you want to use mod.provides, not mod.name
17:49 rubenwardy otherwise you won't detect cycles with mods in modpacks that don't have the same name as the modpack
17:50 MTDiscord  <luatic> So ?type=mod returns modpacks too...
17:50 rubenwardy yeah
17:50 MTDiscord  <luatic> sigh, i need this at a mod level
17:50 rubenwardy CDB doesn't have that info
17:50 MTDiscord  <luatic> yeah
17:50 MTDiscord  <luatic> I'd have to scrape the mods etc.
17:51 MTDiscord  <luatic> But I think just ignoring modpack is good enough for now
17:51 MTDiscord  <luatic> if #mod.provides == 1 and mod.provides[1] == mod.name then ... end
17:52 MTDiscord  <luatic> the "technic" one was a false positive
17:52 MTDiscord  <luatic> so we have VE's couple mods (probably more in modpacks) and someone who doesn't know how to use opt. deps (the legendary guy)
17:57 MTDiscord  <luatic> lol, this code is mutually exclusive: https://github.com/gitgithu/legendary_armor/blob/main/init.lua#L250-L256 and https://github.com/gitgithu/legendary_ore/blob/main/init.lua#L31-L40
17:58 MTDiscord  <luatic> I guess we needn't care about the legendary stuff, it's poorly made - a direct cyclic optional dependency simply is unreasonable.