Plug gazelle_cabal.ImportData in Resolve to allow both gazelle extensions to run in the same gazelle_binary #41
Conversation
…ions to run in the same gazelle_binary
gazelle_haskell_modules/lang.go
Outdated
| // This happens only when one run gazelle_cabal and gazelle_haskell_modules in one pass as the the gazelle_binary. | ||
| case gazelle_cabal.ImportData: | ||
| // We fully generate the rule that gazele_cabal would have produced. | ||
| gazelle_cabal.RunResolve(c, ix, rc, r, expr, from) |
There was a problem hiding this comment.
I need some help understanding the flow. Is this case entered anytime a haskell_binary/library/test rule is updated or created by gazelle_cabal?
If so, how are haskell_module rules generated from this rule?
There was a problem hiding this comment.
This case is entered whenever the Resolve function is called on rules that come from the same gazelle run (so rules where the imports where constructed by gazelle_cabal and not by the haskell lib himportscan included in gazelle_haskell_modules).
Regarding the construction of haskell_modules rules, it is performed just like for other rules by setNonHaskellModuleDeps.
There was a problem hiding this comment.
This code is calling RunResolve, which apparently is a duplicate of the Resolve callback of gazelle_cabal. Isn't that callback invoked by gazelle implicitly?
gazelle_haskell_modules/lang.go
Outdated
| // (this field is either non-existent or filled with data from an outdated run), | ||
| // hence we can safely put an empty map here. | ||
| Modules: make(map[label.Label]bool), | ||
| Srcs: srcs, |
There was a problem hiding this comment.
Is this duplicating the code in addNonHaskellModuleRules that creates HRuleImportData for preexisting non-haskell_module rules?
There was a problem hiding this comment.
Yes, it is. Is there any tips to discover the already implemented functions in our projects? Furthermore, I am not sure I understand clearly what the various arguments of this function represent.
There was a problem hiding this comment.
Is there any tips to discover the already implemented functions in our projects?
I don't think so, other than reading the code :)
Furthermore, I am not sure I understand clearly what the various arguments of this function represent.
Here's an attempt at explaining them:
func addNonHaskellModuleRules(
c *Config,
pkgRoot string,
repo string,
pkg string,
gen language.GenerateResult,
rules []*rule.Rule,
) language.GenerateResult
cis the same config parameter as given toResolvepkgRootroot of the project treerepois the name of the bazel repopkgis the package path inside project treegenis the container to which new generated rules are addedrulesare the rules already present in the BUILD file.
There is this invocation that may give further clues on how the arguments are created. Probably we won't need all of these arguments after factoring out the piece that is needed by both addNonHaskellModuleRules and Resolve.
|
This is missing a test, right? |
It is, I would like to be able to run the current examples before creating new ones. |
| } | ||
| } | ||
| // After this cleaning, the new deps are set. | ||
| setNonHaskellModuleDeps(&hmc, c.RepoRoot, ix, r, newImports, from) |
There was a problem hiding this comment.
Why are the attributes deleted from the old rule?
There was a problem hiding this comment.
Sorry for the very delayed answer.
Those attributes are deleted because Resolve is not a pure function, it alters the rules given to it. What I really want to do is to replace r by newr, but this not possible (in the sense that I do whatever is required on newr, this will not impact r and in the end, newr won't exist outside of the scope of this function). So I have to copy newr into r and for this, I use my knowledge that the difference between the 2 is the existence of some attributes to do it by deletion rather than doing a "deep copy".
There was a problem hiding this comment.
Definitely worth adding a comment.
| name = "io_tweag_gazelle_cabal", | ||
| strip_prefix = "gazelle_cabal-240ad1af8d0d0ca5c3fd3ff7afcdb2f0fe0dbbe2", | ||
| urls = ["https://github.com/tweag/gazelle_cabal/archive/240ad1af8d0d0ca5c3fd3ff7afcdb2f0fe0dbbe2.zip"], | ||
| ) |
There was a problem hiding this comment.
It is a unsatisfying that we now have to bring gazelle_cabal even if the user does not intend to use it. But I don't see how to avoid it.
|
For now, I'm closing the PR for considerations made here. But I think this problem would be an opportunity to follow up with |
An attempt to solve #40
This applies the methodology suggested as the third item in this answer #40 (comment)
Now,
gazelle_haskell_modulesdepends ofgazelle_cabal, allowing us to pattern match on the type ofimportsin theResolvefunction.