mbeckerle commented on a change in pull request #486:
URL: https://github.com/apache/incubator-daffodil/pull/486#discussion_r568610805
##########
File path: build.sbt
##########
@@ -177,7 +182,18 @@ lazy val usesMacros = Seq(
// ignores files such a META-INFA/LICENSE and NOTICE that are duplicated and
// would otherwise cause a conflict
mappings in (Compile, packageBin) ++= mappings.in(macroLib, Compile,
packageBin).value.filter { case (f, _) => f.isDirectory ||
f.getPath.endsWith(".class") },
- mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value
+ mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value,
+
+ // The sbt eclipse plugin does not addd the macroLib project as a dependency
+ // in .classpath files because macroLib isn't actually an sbt dependncy.
+ // Instead, sbt does the above stuff to copy macro lib files into those
Review comment:
What makes macrolib "not an sbt dependency". I mean if you look above at
the lazy val lib, it has
```
.dependsOn(macroLib % "compile-internal, test-internal")
```
which sure looks like an sbt dependency to me. So what do you mean by "isn't
an sbt dependency"?
So macrolib only contains macro definitions, so is treated this way to avoid
a "real" dependency that would otherwise hang around and be required at
runtime? If that's right, let's add that to the comments here.
This page (below) describes sbt macro dependency. Is that what we're doing
here? It's hard to tell reading that page, and reading build.sbt to recognize
if we're using that same technique or not.
If what we're doing is the standard macro thing for sbt, we should say so in
the comments, and if not we should clearly say so and why.
I'm surprised that sbt eclipse doesn't do the right thing if we're using the
standard sbt technique for dealing with macros.
https://www.scala-sbt.org/1.x/docs/Macro-Projects.html
If sbt eclipse isn't handling macros properly, then should we be reporting a
bug in sbt eclipse, along with our "fix" for it? Or is what you have done sbt
eclipse's way of handling this? I.e., it's not automatic, macros require use of
hooks?
##########
File path: build.sbt
##########
@@ -177,7 +182,18 @@ lazy val usesMacros = Seq(
// ignores files such a META-INFA/LICENSE and NOTICE that are duplicated and
// would otherwise cause a conflict
mappings in (Compile, packageBin) ++= mappings.in(macroLib, Compile,
packageBin).value.filter { case (f, _) => f.isDirectory ||
f.getPath.endsWith(".class") },
- mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value
+ mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value,
+
+ // The sbt eclipse plugin does not addd the macroLib project as a dependency
+ // in .classpath files because macroLib isn't actually an sbt dependncy.
+ // Instead, sbt does the above stuff to copy macro lib files into those
+ // projects that need it. Because this copy doesn't happen for eclipse,
+ // eclipse can't find the macroLib files and leads to compilation errors. To
+ // fix this, we add an sbt eclipse transformer that adds the missing macroLib
+ // dependency to .classpath files only for these projects that use the
macroLib
+ EclipseKeys.classpathTransformerFactories ++= Seq(
+ transformNode("classpath",
DefaultTransforms.Append(EclipseClasspathEntry.Project(macroLib.base.toString))),
Review comment:
btw: how does this avoid appending this classpath entry on the classpath
for daffodil-macro-lib itself?
The EclipseClasspathEntry.Project(macrolib.base.toString) seems like the
classpath entry to create. Not a filter.
Nothing else seems to filter out particular subprojects.
Where is that specified?
##########
File path: build.sbt
##########
@@ -177,7 +182,18 @@ lazy val usesMacros = Seq(
// ignores files such a META-INFA/LICENSE and NOTICE that are duplicated and
// would otherwise cause a conflict
mappings in (Compile, packageBin) ++= mappings.in(macroLib, Compile,
packageBin).value.filter { case (f, _) => f.isDirectory ||
f.getPath.endsWith(".class") },
- mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value
+ mappings in (Compile, packageSrc) ++= mappings.in(macroLib, Compile,
packageSrc).value,
Review comment:
Typo "classe" in comment above.
##########
File path: build.sbt
##########
@@ -15,6 +15,11 @@
* limitations under the License.
*/
+// quiet errant sbt linter warnings about unused sbt settings
Review comment:
Why is this needed? Why did "linter" and Global / excludeLintKeys get
involved in this?
The comment says what it does, but not why this is needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
