-
Notifications
You must be signed in to change notification settings - Fork 446
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
Cross build to sbt 2.0.0-M3 #1647
Conversation
f7a49bf
to
e90f8a0
Compare
I am fixing scripted tests in eed3si9n#1 |
b6e5599
to
1b7a7b2
Compare
c9d85e1
to
32ed998
Compare
I will try to have a look in the coming days |
# Workaround: set target folder to what it was in sbt 1.x because with sbt 2.x and project matrix target is target/out/jvm/scala-3.3.3/ | ||
> set target := baseDirectory.value / "target" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a few places with this workaround. Lets remove it and use exists
with glob?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept this one because
> checkSoftlink target/debian-test-0.1.0/usr/share/debian-test/logs points to /non-standard/log/debian-test
contains a relative path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. Also in src/sbt-test/docker/entrypoint/test
I see
exec grep -q -F 'ENTRYPOINT ["/bin/sh", "-c", "env"]' target/docker/stage/Dockerfile
which we can't change to use the glob.
However there are still 28 references to this workaround, and I think we some one them can be changed to use the glob like src/sbt-test/debian/upstart-deb-facilities/test
or src/sbt-test/docker/staging/test
for example....
implicit val converter: FileConverter = fileConverter.value | ||
val assetsDir = baseDirectory.value / "src" / "main" / "assets" | ||
assetsDir.**(AllPassFilter).filter(_.isFile).classpath.map( | ||
_ | ||
.put(PluginCompat.artifactStr, PluginCompat.artifactToStr((Assets / artifact).value)) | ||
.put(PluginCompat.moduleIDStr, PluginCompat.moduleIDToStr(projectID.value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall correctly, this was migrated from using a directory ( src/main/assets
) to all the files in the directory, because the new HashedVirtualFileRef
cannot be used with directories...
import sbt.* | ||
import xsbti.FileConverter | ||
|
||
object PluginCompat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no longer private. should we consider it part of this plugin public API? Or somehow move it to an internal package (like com.typesafe.sbt.packager.internal
) to signal it should not be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the type aliases needs to be public since they are part of keys, but individual functions should be scoped to private[packager]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added private[packager]
in 2a20c60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already looking good. Thanks a tone @eed3si9n ❤️ ❤️
Would you consider this ready to merge?
Sort of yes/no. Overall this PR shows that we're close, but @jtjeferreira has provided a few feedbacks that I would like to address this weekend. |
3e0aefe
to
f1de9e7
Compare
**Problem/Solution** This cross builds sbt-native-packager for sbt 1.x and 2.0.0-M3. Co-authored-by: Eugene Yokota <[email protected]> Co-authored-by: João Ferreira <[email protected]>
ok. I think this is ready to land. |
|
Nice. I'm going to hand-edit the release note. Here's the generated one as back up - https://gist.github.com/eed3si9n/6c26c9ca0d711277e76f32e7c97ab795 |
Fixes #1638
Problem/Solution
This cross builds sbt-native-packager for sbt 1.x and 2.0.0-M3.
This PR was co-authored by @eed3si9n and @jtjeferreira.