trying to find the best practices after

[cunning grail]

The goal of that change was to steer module authors towards making directories/files that are outside their source code dir into explicit arguments to the module, in order to reduce the amount of "shadow arguments and dependencies" that having access to the full root enables.

Let me look at the structure of that repo though and see if there's any adjustments that would make sense

[muted hawk]

I was inspired by the way @quaint raptor implemented his modules tests here: https://github.com/sagikazarmark/daggerverse/tree/main/ci - which I like, but is now broken.

loading local modules seems a no-brainer when implementing tests for other modules, you want your CI code to share the same git ref

An alternative would be to include the ci code inside each module, but it exposes the functions to the top level, which is very verbose...

[cunning grail]

Yeah there's a way of adjusting the structure of your repo to have those local dependencies without putting the source in the root, I'll make the change as an example, one min

Just sent as a PR so you can look at the updated code easiest: https://github.com/samalba/dagger-modules/pull/1

Basically, just moved what was in the root of the repo to be in a subdir ci/. The ci module can still depend on the other local module though (you'll see in its dagger.json that it has a dep on "../modules/inline-python")

The commands I ran were:

sipsma@dagger_dev:sam-dagger-modules$ rm -rf dagger.* go.* main.go LICENSE querybuilder/
sipsma@dagger_dev:sam-dagger-modules$ dagger mod -m ci init --name=ci --sdk=go
WARNING: no LICENSE file found; generating one for you, feel free to change or remove license="Apache-2.0"                                                          
• Engine: 96ebbc1e0b32 (version v0.9.8)
⧗ 4.00s ✔ 70 ∅ 3
sipsma@dagger_dev:sam-dagger-modules$ dagger mod -m ci install modules/inline-python/
• Engine: 96ebbc1e0b32 (version v0.9.8)
⧗ 4.69s ✔ 131 ∅ 3
sipsma@dagger_dev:sam-dagger-modules$ dagger call -m ci run-tests
✔ dagger call run-tests [4.86s]
┃ true
• Engine: 96ebbc1e0b32 (version v0.9.8)
⧗ 8.87s ✔ 142 ∅ 8

[muted hawk]

I see that, and that the root-for key got removed from the dagger.json

not super intuitive to be honest, but I am glad it's possible at least

[cunning grail]

The lack of docs yet are the biggest impediment, but yeah we're going to do another pass on this (discussed here https://discord.com/channels/707636530424053791/1202321084016513044), which should polish it up more

What specifically was un-intuitive though? I can guess, but rather hear from you 🙂

[austere plover]

root-for will still not allow you to access the app source code implicitly

[muted hawk]

well, I was expecting not to need -m, and install straight with dagger mod install ../my/mod which tells me it cannot escape the root. So basically telling the user "no that's forbidden, forget that"

[cunning grail]

Yeah agree the -m during the init is in need of something more explicit. There's logical justification for it underneath but it doesn't bubble up to the UX atm.

One thing to clarify is that after the dagger mod -m ci init --name=ci --sdk=go it would also have been valid to do this:

sipsma@dagger_dev:sam-dagger-modules$ cd ci/
sipsma@dagger_dev:sam-dagger-modules$ dagger mod install ../modules/inline-python/
• Engine: 96ebbc1e0b32 (version v0.9.8)
⧗ 4.69s ✔ 131 ∅ 3
sipsma@dagger_dev:sam-dagger-modules$ dagger call run-tests
✔ dagger call run-tests [4.86s]
┃ true
• Engine: 96ebbc1e0b32 (version v0.9.8)
⧗ 8.87s ✔ 142 ∅ 8

That would result in the exact same thing
Basically, once you've done the init that sets the root of the module, the rest of the commands work with relative paths as long as they don't escape that root

[muted hawk]

got it, that makes sense. But a bit challenging to explain 😛

[austere plover]

@muted hawk if you just want to have a top-level CI module for your app repo, without polluting your repo root with the module's source code (basically the standard CI use case), this will be the best practice next week:

• Go to the root of the repo
• dagger init --name NAME (initialize Dagger module)
• dagger develop --sdk go --source ./ci (develop functions for your module, with source code at the given path)

[muted hawk]

not sure about introducing a new command for that (develop), I find it elegant to have your ci code be a regular module with local dependencies. my 2c

[austere plover]

The dagger.json will be at the root of the repo, not in ci

[austere plover]

Yes your CI code will still be a regular module, and it can still have local dependencies.

For example in the Dagger repo, the root module would probably import each SDK sub-module:

dagger install ./sdk/go ./sdk/python ./sdk/typescript

etc

It's true that if any of my modules needs to import ../../foo, getting an error, and having to realize you need to add root-for to the right parent directory, is inconvenient. Or @cunning grail did you plan on automatically doing all that at dagger install?

[cunning grail]

Not during dagger install no, I was thinking we'd add support for changing the root in an explicit command (e.g. maybe dagger config set root-for ../../ or dagger config re-root ../../, etc.).

But was going to save that in particular for follow ups down the line. Next week will have the adjustments to support --source, also being able to re-root might be in the cards but unlikely just in terms of time next week (will require tons of tests around the millions of corner cases, etc.)

[quaint raptor]

I really appreciate the release notes @cunning grail , but yeah…. the new structure is not super intuitive and I haven’t been able to update my modules yet.

Still working on the conversion and I’m not sure my current daggerverse will be able to continue as it is

(See latest PR)

[austere plover]

Want to go over it live next week? The implementation is not quite finished, some of the confusion comes from that

• after the conversation above we found a way to simplify it further

[quaint raptor]

Definitely!

[austere plover]

There were a LOT of edge cases and user requests to triangulate into a design that works for everyone. I think the fundamentals are sound, once the polish is there it will look and feel better

[quaint raptor]

I’ve been on the road all week and Dagger doesn’t really work offline, but after FOSDEM this weekend I’ll definitely spend some time on the new structure.

My immediate feedback is that I couldn’t convert my modules to the new structure easily on the road.

Again, I do appreciate the “release note” msg in the channel 🙏

[quaint raptor]

Doesn’t mean it’s bad though.

[cunning grail]

Yeah Solomon and I chatted a bit after this, I think the updates I'm working on now to the implementation will be a huge improvement and result in users not having to think about root anymore. Essentially, "root" will always be the root of the repo and modules can have dependencies on anything under that root with needing to run init -m in the exact right place (which I think is the biggest problem right now).

The trickiness is that the engine needs to be much more clever about sparsely loading only the files/dirs it needs, but by coincidence I needed to implement 1/2 of that for bug fix yesterday, so I'm reusing that to improve this situation now too 😄

[quaint raptor]

I’m less concerned about an initial init command.
The mental model and the intuitiveness is more important IMO.

I even get the rationale behind moving to the new structure.

I’ve grown to like the sandbox model. (Lopkong forward to easier env var use though)

That root-for bit is not easy to understand and apparently it’s already deprecated? 😃

In any case, I think the pattern I came up with for testing modules is a reasonable one. I’d love to see something similar to live on.

[cunning grail]

Yeah to be clear we're not changing the sandboxing or just giving the module code itself full access to the entire repo, the change I'm working on now is just around the loading, root-for, etc. Which were supposed to be internal-only concerns but I think leaked too much into the UX.

It's probably best to just finish it and discuss then, this topic has proven itself to be hard to talk about in the abstract at times 😄

[cunning grail]

Sort of, but in a backwards compatible way this time 🙂