#Differences between +ignore and .gitignore

1 messages ยท Page 1 of 1 (latest)

lapis cape
#

One example I found

#

Example 1

  • Pragma: +ignore=["**", "!.github/workflows"]
  • Expected: everything is ignored except for .github/workflows
  • Actual: everything is ignored

Example 2

  • Pragma: +ignore=["!.github/workflows"]
  • Expected: nothing is ignored
  • Actual: everything is ignored except for .github/workflows
#

@tall viper

tall viper
#

Normally only .github should be there, but if you do this you don't need to put **

#

It's because ! == include, if it's include then the rest is ignored, that's the way the buildkit API do

lapis cape
#

but that's not the same behavior as .gitignore (I think)

#

remember when I said "don't underestimate the difficulty of getting the ignore syntax right" ๐Ÿ˜›

#

I know buikdkit works differently. That's why it's extra work because we want to hide the buildkit behavior. we want the gitignore behavior

tall viper
#

Yeah but buildkit include behave as include only, I wonder how we could workaround that

lapis cape
#

I tried to warn you 2 months ago

tall viper
#

I know, but I don't think it was worth it to hold the PR just for that, it can be done in a follow up

lapis cape
#

I can relate to that

tall viper
#

@covert heath Do you have any idea how we can handle this though?

lapis cape
#

But we can't wait too long for the follow-up, because once developers start relying on the current behavior, it will break them when we change it

tall viper
#

Because it's quite complex, we cannot use the Include field of LocalImport if we want to keep the same .gitignore logic, or maybe there are changes to be done on this part?

lapis cape
#

I don't know for sure, to have a clear answer we first need an in-depth understanding of how buildkit's include/exclude implementation works in the edge cases, it's pretty obscure and poorly documneted

covert heath
#

Yeah I have to relearn the quirks every time I interact with buildkit's filtering logic.

I am trying to close a bunch of threads pre-PTO right now, probably won't have time to deep dive this today but I can at least try to point you to the right places in buildkit code if that would be helpful @tall viper?

tall viper
#

Yeah definitely!

covert heath
#

Okay will get back by EOD, finishing a couple other things first ๐Ÿ™‚

lapis cape
#

@tall viper since it's pure buildkit, you can also try the buildkit channel in the dagger community slack

covert heath
#

@tall viper The entrypoint from dagger code to all this is here: https://github.com/sipsma/dagger/blob/24c5576e6fd83938bd7e5db262a85f96f67c916d/engine/client/filesync.go#L145-L163

That's the code in the client (e.g. the CLI) that receives the options in terms of paths, include/exclude etc. and then passes them off to fsutil, which sends the files/dirs back to the server.

In fsutil, the github.com/moby/patternmatcher lib is used (https://github.com/sipsma/fsutil/blob/91a3fc46842c58b62dd4630b688662842364da49/filter.go#L111-L138) but there's quite a bit of intricate logic layered on top (e.g. https://github.com/sipsma/fsutil/blob/91a3fc46842c58b62dd4630b688662842364da49/filter.go#L219-L252)

I can't say off the top of my head whether it's possible to replicate .gitignore style matching with this logic, but some things to keep in mind:

  1. It's pretty important for performance to keep the pattern matching on the client-side. It would be more convenient to do it server side but that'd require loading the whole dir w/ no filtering into the engine first
  2. There is this Map callback that we provide to fsutil that can be used to choose whether to include/exclude individual paths. Maybe there's a route to implementing our own filtering logic through that? https://github.com/sipsma/dagger/blob/24c5576e6fd83938bd7e5db262a85f96f67c916d/engine/client/filesync.go#L154
  3. We shouldn't rule out upstream tweaks to make this more customizable if needed either; just want to make sure everything is backwards compatible and doesn't impact buildkit in general
covert heath
tall viper