I hope I'm wrong, but my first suspicion | Dagger | Page 1

atomic basalt Sep 20, 2024, 11:01 PM

#

Yeah locally this diff fixes the problem:

diff --git a/version/dagger.json b/version/dagger.json
index b47eb30af..ecf01512d 100644
--- a/version/dagger.json
+++ b/version/dagger.json
@@ -3,10 +3,10 @@
   "sdk": "go",
   "dependencies": [
     {
-      "name": "git",
+      "name": "coolgit",
       "source": "github.com/shykes/git@4f46bfde5735e17612316a30bc5a43c3eefd6da2"
     }
   ],
   "source": ".",
-  "engineVersion": "v0.13.0"
+  "engineVersion": "v0.13.1"
 }
diff --git a/version/go.mod b/version/go.mod
index ec36834d2..f430805d3 100644
--- a/version/go.mod
+++ b/version/go.mod
@@ -17,6 +17,7 @@ require (
        go.opentelemetry.io/otel/trace v1.27.0
        go.opentelemetry.io/proto/otlp v1.3.1
        golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa
+       golang.org/x/mod v0.18.0
        golang.org/x/sync v0.8.0
        google.golang.org/grpc v1.66.1
 )
diff --git a/version/go.sum b/version/go.sum
index 40f88b0b1..c9e3fee13 100644
--- a/version/go.sum
+++ b/version/go.sum
@@ -63,6 +63,8 @@ go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
 golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa h1:FRnLl4eNAQl8hwxVVC17teOw8kdjVDVAiFMtgUdTSRQ=
 golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa/go.mod h1:zk2irFbV9DP96SEBUUAy67IdHUaZuSnrz1n472HUCLE=
+golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
+golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo=
 golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0=
 golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
diff --git a/version/main.go b/version/main.go
index 39cbdb91d..484bb7123 100644
--- a/version/main.go
+++ b/version/main.go
@@ -82,8 +82,8 @@ func (v Version) Version(ctx context.Context) (string, error) {
        return fmt.Sprintf("%s-%s-%s", next, commit, digest), nil
 }
 
-func (v Version) gitRepo() *dagger.GitRepo {
-       return dag.Git().Load(v.GitDir, dagger.GitLoadOpts{
+func (v Version) gitRepo() *dagger.CoolgitRepo {
+       return dag.Coolgit().Load(v.GitDir, dagger.CoolgitLoadOpts{
                Worktree: v.Inputs,
        })
 }

#

@trail carbon (started a thread)

#

Basically the diff just changes the name of the dependency to avoid the conflict with the core type

trail carbon Sep 20, 2024, 11:02 PM

#

damn

#

But this worked before 😭

atomic basalt Sep 20, 2024, 11:02 PM

#

So it at least has a pretty easy workaround

trail carbon Sep 20, 2024, 11:02 PM

#

I've used this module before - but perhaps never from code?

atomic basalt Sep 20, 2024, 11:03 PM

#

It could arise suddenly due to changes elsewhere. Basically what's happening is you are receiving a Directory as an arg. But everything is just a dagql ID, which is just a gql query. So your module is resolving the arg, which has a call to Query.git in it and then it hits your dependency instead of the core API

#

But previously if you didn't pass a Directory arg where there was an embedded Query.git call, the problem would never get hit

trail carbon Sep 20, 2024, 11:05 PM

#

I see, so the same ID might be resolved differently based on the receiver's dependency namespace?

#

Kind of feels like the ID should resolve the name of the underlying module at the time of ID creation?

#

but I guess that's a whole can of worms

atomic basalt Sep 20, 2024, 11:06 PM

#

trail carbon I see, so the same ID might be resolved differently based on the receiver's depe...

Yeah in the receiver the core Git api got overridden by your dependency, so the dependency was called instead (and it doesn't know about keepGitDir)

atomic basalt Sep 20, 2024, 11:06 PM

#

trail carbon Kind of feels like the ID should resolve the name of the underlying module at th...

Yeah this is for sure a bug, what you're describing is the expected behavior

trail carbon Sep 20, 2024, 11:06 PM

#

I see, this was allowed to happen because it was done by the CLI - I'm guessing if I tried to do the same thing from a module, the SDK would prevent me with typechecking

#

(ie. the SDK would not allow my module to make that particular function call with that particular ID, even though at runtime it would "work")

#

actually no that's wrong. Sorry I'm getting confused.

#

Anyway thanks for finding the fix 🙂

atomic basalt Sep 20, 2024, 11:28 PM

#

Well apparently I opened an issue about this last December 😅 Left a more modern comment explaining why it's extra confusing and how to fix potentially https://github.com/dagger/dagger/issues/6328#issuecomment-2364745799

GitHub

Fix module name conflicts with core types · Issue #6328 · dagger/da...

Related discord question: #1190052885686407179 message Right now a module that has the same name as a core type (e.g. Module, Container, e...

trail carbon Sep 20, 2024, 11:42 PM

#

@atomic basalt quick follow-up, this fails with the same error... But it's my git module itself making the call.. so not sure what diff would fix it in this case. Also confused about why the git module itself would resolve the wrong git?

#

dagger -m github.com/shykes/git call load --source=https://github.com/dagger/dagger#sdk/python/v0.11.0 command --args=tag,--points-at,HEAD stdout

Output:

error: parse selections: parse field "git": Query.git has no such argument: "keepGitDir"

atomic basalt Sep 20, 2024, 11:46 PM

#

Ah shit, that makes sense sort of. Even though SDKs don't have self calls, the API itself does support it. So when you make the Query.git call you are actually calling your own API 😵‍💫

Did this github.com/shykes/git module work recently? I'm starting to wonder if something changed where this used to actually work one way or another but doesn't now

trail carbon Sep 20, 2024, 11:46 PM

#

I haven't used it in a while, before today

#

It used to work fine - but probably still requires an old engine version

#

So maybe a compat mode issue?

atomic basalt Sep 20, 2024, 11:48 PM

#

trail carbon So maybe a compat mode issue?

that's the biggest change to how queries get resolved that was made recently, so definitely a suspect

#

but hard to say for sure

#

I'll look some more

#

The only workaround in the meantime is unfortunately to rename your module itself to something besides Git 😞

trail carbon Sep 20, 2024, 11:51 PM

#

https://tenor.com/view/me-sad-me-very-sad-sad-gif-14435084487592172410

Tenor

#

It was originally called supergit - at some point I renamed it, not sure if something happened that made that possible

atomic basalt Sep 21, 2024, 12:09 AM

#

I went back to v0.11.2 (when the last commit to the git module was pushed) and the example call you gave failed the same, same even on v0.10.3, so I am not sure if there was a brief window of time where it worked or maybe it just works sometimes depending on the call you make. Either way, doesn't appear to be a compat mode issue

#

But either way, that definitely worsens the bug since this is much much more annoying to fix

trail carbon Sep 21, 2024, 12:18 AM

#

Yeah it's possible I never actually used load against a remote directory

#

It makes sense actually, since this module has its own implementation of remote fetch..

#

So it's just an edge case I had never hit before

atomic basalt Sep 21, 2024, 12:19 AM

#

Ahhh okay yeah, that does make perfect sense then...

#

I'm gonna give a quick shot at a simple fix for just the "self API call" case, I think if we make it so the API always installs self module calls at Query.self rather than Query.<modname> that would at least avoid this (and also be aligned with upcoming support for dag.Self most likely?)

#

Possible everything will blow up but worth a try

trail carbon Sep 21, 2024, 12:23 AM

#

atomic basalt Possible everything will blow up but worth a try

I need to start writing down all the T-shirt opportunities

#I hope I'm wrong, but my first suspicion