Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce CompileTime custom configuration #415

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

satorg
Copy link
Contributor

@satorg satorg commented Nov 13, 2022

Resolves #409

@satorg satorg self-assigned this Nov 13, 2022
@satorg satorg requested a review from armanbilge November 13, 2022 08:38
@mergify mergify bot added the kernel label Nov 13, 2022
@@ -29,6 +29,8 @@ object TypelevelKernelPlugin extends AutoPlugin {
override def trigger = allRequirements

object autoImport {
lazy val CompileTime = config("compile-time").hide
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I have no idea what that .hide does 🤷
I failed to find out any docs about it.
I tried to ask here, but no luck either.

However, I managed to verify it locally by creating a test project with the scalac-compat-annotation library added as CompileTime dependency. Then I checked that publishLocal generates POMs that do no include that library. Seems working.

Still not sure if it is better with .hide rather than without it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's invoking this method.
https://github.com/sbt/librarymanagement/blob/436656f71923960ec99d3c8b31ef8d9984dbbb74/core/src/main/scala/sbt/librarymanagement/ConfigurationExtra.scala#L104

which is setting isPublic to false
https://github.com/sbt/librarymanagement/blob/436656f71923960ec99d3c8b31ef8d9984dbbb74/core/src/main/scala/sbt/librarymanagement/Configuration.scala#L70

which I believe is modifying the visibility attribute:

visibility is used to indicate whether or not a configuration can be used from other modules depending on this one. Thus a private configuration is only used for internal purpose (maybe at build time), and other modules cannot declare to depend on it.

https://ant.apache.org/ivy/history/latest-milestone/ivyfile/conf.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an explicit type to this? Trying to maintain binary-compatibility with auto-inferred types always makes me nervous 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's invoking this method.

Yes, it was the easiest part of my investigation :) , but unfortunately isPublic is not somehow documented nor even commented in SBT.

which I believe is modifying the visibility attribute:

Well, for me it was not that clear though, but at least your assumption makes some sense. So according to that I think we can keep .hide since we're not going to share this configuration with other modules, correct?

Copy link
Contributor Author

@satorg satorg Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@armanbilge

which I believe is modifying the visibility attribute:

Actually, now I'm not that sure about it... It does not look like that: I've tried both options (with and without .hide applied) and in both cases the compile-time conf gets visibility=public in both sbt-typelevel ivy as well as the test project one that adds the sbt-typelevel plugin to its plugins.sbt. For example, this is ivy.xml generated for my test project that receives the compile-time conf from the plugin settings:

<?xml version="1.0" encoding="UTF-8"?>
<ivy-module version="2.0" xmlns:e="http://ant.apache.org/ivy/extra">
  <info organisation="default" module="sbt-typelevel-test-compile-only_2.13" revision="0.1-M4" status="release" publication="20221113190746" e:info.versionScheme="early-semver">
    <description>sbt-typelevel.test.compile-only</description>
  </info>
  <configurations>
    <conf name="plugin" visibility="public" description=""/>
    <conf name="pom" visibility="public" description=""/>
    <conf extends="runtime" name="test" visibility="public" description=""/>
    <conf name="provided" visibility="public" description=""/>
    <conf extends="compile,optional,provided" name="compile-internal" visibility="public" description=""/>
    <conf name="compile-time" visibility="public" description=""/>
    <conf name="docs" visibility="public" description=""/>
    <conf name="optional" visibility="public" description=""/>
    <conf name="compile" visibility="public" description=""/>
    <conf extends="test,optional,provided" name="test-internal" visibility="public" description=""/>
    <conf name="scala-tool" visibility="public" description=""/>
    <conf name="scala-doc-tool" visibility="public" description=""/>
    <conf name="sources" visibility="public" description=""/>
    <conf extends="compile" name="runtime" visibility="public" description=""/>
    <conf extends="runtime,optional" name="runtime-internal" visibility="public" description=""/>
  </configurations>
  <publications>
    <artifact name="sbt-typelevel-test-compile-only_2.13" type="jar" ext="jar" conf="compile"/>
    <artifact e:classifier="sources" name="sbt-typelevel-test-compile-only_2.13" type="src" ext="jar" conf="sources"/>
    <artifact e:classifier="javadoc" name="sbt-typelevel-test-compile-only_2.13" type="doc" ext="jar" conf="docs"/>
    <artifact name="sbt-typelevel-test-compile-only_2.13" type="pom" ext="pom" conf="pom"/>
  </publications>
  <dependencies>
    <dependency org="org.scala-lang" name="scala-compiler" rev="2.13.10" conf="scala-tool->default"> </dependency>
    <dependency org="org.scala-lang" name="scala-compiler" rev="2.13.10" conf="scala-tool->optional(default)"> </dependency>
    <dependency org="org.scala-lang" name="scala-library" rev="2.13.10" conf="scala-tool->default"> </dependency>
    <dependency org="org.scala-lang" name="scala-library" rev="2.13.10" conf="scala-tool->optional(default)"> </dependency>
    <dependency org="org.scala-lang" name="scala-library" rev="2.13.10" conf="compile->default(compile)"> </dependency>
    <dependency org="com.olegpy" name="better-monadic-for_2.13" rev="0.3.1" conf="plugin->default(compile)"> </dependency>
    <dependency org="org.typelevel" name="kind-projector_2.13.10" rev="0.13.2" conf="plugin->default(compile)"> </dependency>
    <dependency org="org.typelevel" name="scalac-compat-annotation_2.13" rev="0.1.0" conf="compile-time->default(compile)"> </dependency>
  </dependencies>
</ivy-module>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an explicit type to this?

Totally 👍 . Done, please take a look.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol ... of course it doesn't 😂 well, thanks for checking that. Hmm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... me too 🤔

Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so idk what is going on with ivy ...

But I checked the POM with/without the .hide, and it seems to me that the .hide prevents the dependency from appearing in the POM, which is exactly what we want.

@satorg feel free to merge, I feel pretty good enough about this and more importantly I have need for it 😅 Thanks for all your work!

@armanbilge
Copy link
Member

Then I checked that publishLocal generates POMs that do no include that library. Seems working.

Ah, that's exactly what you said :) so, good enough for me.

@satorg
Copy link
Contributor Author

satorg commented Nov 14, 2022

it seems to me that the .hide prevents the dependency from appearing in the POM, which is exactly what we want.

Well, the matter is that when I gave it a try, then both options – with and without .hide did their job in the same way.
So that confused me a lot.

Perhaps, I'll re-check it again later to make sure that I did no mistake, but I think that since it apparently works with .hide, then we can leave it as is for now to keep moving forward. It is not that important after all.

And thank you for looking into this, it is so helpful!

@satorg satorg merged commit 0977b12 into typelevel:series/0.4 Nov 14, 2022
@satorg satorg deleted the add-compile-time-config branch November 14, 2022 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CompileTime to sbt-typelevel-kernel
2 participants