stevedlawrence commented on a change in pull request #486:
URL: https://github.com/apache/incubator-daffodil/pull/486#discussion_r568740275
##########
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:
Yep, that page describes exactly what we're doing, and probably where we
got the idea to do this, and I think is the "right" way to handle internal
macros. We don't really need to publish a jar of macros since they are internal
to Daffodil. So we disabling publishing that jar, and copy the macro code into
projects that use macros.
I can adjust the comment to make this distinction.
Yeah, it does sound like a bug in eclipse. Unfortunately, sbt eclipse plugin
hasn't been updated for two years, so I'm not sure how likely we are to get
this bug fixed. I imagine the fix is for eclipse to just treat these
compile-internal dependencies as if they were compile dependencies (which is
basically what we're doing)--I'm not sure if eclispe has a concept of
"compile-internal"
##########
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:
{code:scala}
transformNode("classpath",
DefaultTransforms.Append(EclipseClasspathEntry.Project(macroLib.base.toString)))
{code}
This line says to transform the "classpath" node. And the transformation it
will apply is "Append" to children. And it will append a ClassPathEntry
Project. So this line appends new ``<classpathentry... >`` dependency for the
macro lib project.
As to how this doesn't apply this transformation for the macro-lib project
itself, this setting is in the "usesMacros" group of settings. So any projects
that uses macros need to add the compile-internal dependency to macro lib, and
add the "usesMacros" settings. For example, daffodil-io looks like this:
{code}
lazy val io = Project("daffodil-io",
file("daffodil-io")).configs(IntegrationTest)
.dependsOn(lib, macroLib % "compile-internal,
test-internal")
.settings(commonSettings, usesMacros)
{code}
So only projects that have usesMacros will get this transformation and will
add the dependency to eclipse files. Since macro-lib doesn't have usesMacro,
the depenceny isn't added. In fact, this now means only add this classpathentry
to daffodil-lib, daffodil-io, and daffodil-runtime1 (the only things that has
usesMacros) so most eclipse projects now won't have this depenency added.
Probably not a big deal technically, but maybe has the potential to speed up
compiling with one less depenency?
{code}
##########
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:
```scala
transformNode("classpath",
DefaultTransforms.Append(EclipseClasspathEntry.Project(macroLib.base.toString)))
```
This line says to transform the "classpath" node. And the transformation it
will apply is "Append" to children. And it will append a ClassPathEntry
Project. So this line appends new ``<classpathentry... >`` dependency for the
macro lib project.
As to how this doesn't apply this transformation for the macro-lib project
itself, this setting is in the "usesMacros" group of settings. So any projects
that uses macros need to add the compile-internal dependency to macro lib, and
add the "usesMacros" settings. For example, daffodil-io looks like this:
```scala
lazy val io = Project("daffodil-io",
file("daffodil-io")).configs(IntegrationTest)
.dependsOn(lib, macroLib % "compile-internal,
test-internal")
.settings(commonSettings, usesMacros)
```
So only projects that have usesMacros will get this transformation and will
add the dependency to eclipse files. Since macro-lib doesn't have usesMacro,
the depenceny isn't added. In fact, this now means only add this classpathentry
to daffodil-lib, daffodil-io, and daffodil-runtime1 (the only things that has
usesMacros) so most eclipse projects now won't have this depenency added.
Probably not a big deal technically, but maybe has the potential to speed up
compiling with one less depenency?
##########
File path: build.sbt
##########
@@ -15,6 +15,11 @@
* limitations under the License.
*/
+// quiet errant sbt linter warnings about unused sbt settings
Review comment:
Newer versions of sbt adding some built-in linter to validate sbt
configurations (likely because it's so hard to know if sbt is doing the right
thing). WIthout this it complains the ``classpathTransformerFactories`` aren't
actually used. They are clearly being used (the .classpath files do have the
correct macro lib dependency), so I think this is just a case where the linter
can't check something correctly. It ends up spitting out a bunch of noise, and
reccomends this as the solution to quiet it.
##########
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:
WIll fix
----------------------------------------------------------------
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]
