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 UberJarMergedResourceBuildItem and UberJarIgnoredResourceBuildItem #17077

Merged
merged 2 commits into from
May 11, 2021

Conversation

gastaldi
Copy link
Contributor

@gastaldi gastaldi commented May 7, 2021

Fixes #5677

gsmet
gsmet previously requested changes May 7, 2021
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I haven't looked at it in detail but it would be better to name it UberJarMergedResourceBuildItem.

The way it is right now, it looks like an event that is triggered once resources are merged.

@gastaldi gastaldi force-pushed the resource_merge_item branch from 2631d23 to b5cb14f Compare May 7, 2021 19:06
@gastaldi gastaldi force-pushed the resource_merge_item branch from b5cb14f to 2729dc4 Compare May 7, 2021 19:06
@gastaldi gastaldi changed the title Introduce UberJarResourceMergedBuildItem Introduce UberJarMergedResourceBuildItem and UberJarIgnoredResourceBuildItem May 7, 2021
@gastaldi gastaldi changed the title Introduce UberJarMergedResourceBuildItem and UberJarIgnoredResourceBuildItem Introduce UberJarMergedResourceBuildItem and UberJarIgnoredResourceBuildItem May 7, 2021
@gastaldi gastaldi marked this pull request as ready for review May 7, 2021 19:07
@gastaldi gastaldi requested a review from gsmet May 7, 2021 19:07
@quarkus-bot
Copy link

quarkus-bot bot commented May 7, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 2729dc4

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build Build ⚠️ Check → Logs Raw logs

@gastaldi gastaldi force-pushed the resource_merge_item branch from 2729dc4 to ae4fd89 Compare May 7, 2021 19:15
@gastaldi gastaldi dismissed gsmet’s stale review May 7, 2021 20:20

Changes addressed

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

It would be nice to have some kind of test for this

@gastaldi
Copy link
Contributor Author

gastaldi commented May 8, 2021

It would be nice to have some kind of test for this

Definitely! Can you point me to a test that uses Build Items for reference?

@geoand
Copy link
Contributor

geoand commented May 8, 2021

See

for an example of customizing the build for a test

@gastaldi
Copy link
Contributor Author

gastaldi commented May 8, 2021

@geoand hm, that doesn't seem to work for building Uber JARs 😞 Should I use QuarkusProdModeTest with setRun(true) instead?

@gastaldi
Copy link
Contributor Author

gastaldi commented May 8, 2021

More specifically, this test doesn't work:

package io.quarkus.deployment.pkg.builditem;

import java.io.IOException;
import java.net.URL;
import java.util.Collections;
import java.util.Enumeration;
import java.util.function.Consumer;

import io.quarkus.bootstrap.model.AppArtifact;
import io.quarkus.builder.BuildChainBuilder;
import io.quarkus.test.QuarkusUnitTest;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import static org.assertj.core.api.Assertions.assertThat;

class UberJarMergedResourceBuildItemTest {

    @RegisterExtension
    static QuarkusUnitTest runner = new QuarkusUnitTest()
            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class))
            .withConfigurationResource("application-uber-jar.properties")
            .setForcedDependencies(
                    Collections.singletonList(
                            new AppArtifact("org.apache.cxf", "cxf-rt-transports-http", "3.4.3")
                    ))
            .addBuildChainCustomizer(buildCustomizer());

    @Test
    public void testResourceWasMerged() throws IOException {
        List<URL> resources = Collections.list(getClass().getClassLoader().getResources("META-INF/cxf/bus-extensions.txt"));
        assertThat(resources).hasSize(1);
    }

    static Consumer<BuildChainBuilder> buildCustomizer() {
        return (buildChainBuilder ->
                buildChainBuilder
                        .addBuildStep(context ->
                                              context.produce(new UberJarMergedResourceBuildItem("META-INF/cxf/bus-extensions.txt")))
                        .produces(UberJarMergedResourceBuildItem.class)
                        .build()
        );
    }
}

@geoand
Copy link
Contributor

geoand commented May 8, 2021

Ah darn, you are right.

We would likely need to add the same capability to the prod mode test.

@gastaldi
Copy link
Contributor Author

gastaldi commented May 8, 2021

Hm, QuarkusProdModeTest doesn't seem to work either. I'm adding my test to the core/test-extension/deployment because I can't add it to core/deployment (it would introduce a circular reference to quarkus-junit5-internal) but then it breaks other build steps 😛
This is painful.

@gastaldi
Copy link
Contributor Author

It would be nice to backport this feature so our Quarkiverse maintainers could use it in the 1.13.x versions

@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 12f58ac

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build Build ⚠️ Check → Logs Raw logs

@gsmet
Copy link
Member

gsmet commented May 10, 2021

There is an import issue.

Next 1.13 (and probably last) will be released on Wednesday morning.

gsmet
gsmet previously requested changes May 10, 2021
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

The imports need a fix and the tests could be improved a bit. If this is done before tomorrow evening, I can backport it. Thanks!

@gastaldi gastaldi force-pushed the resource_merge_item branch from 12f58ac to cc06b7b Compare May 10, 2021 13:19
@gastaldi gastaldi requested review from gsmet and geoand May 10, 2021 13:20
@gastaldi gastaldi force-pushed the resource_merge_item branch from cc06b7b to cc2d7a9 Compare May 10, 2021 13:25
@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2021

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building cc06b7b

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build Build ⚠️ Check → Logs Raw logs

@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 10, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building cc2d7a9

Status Name Step Test failures Logs Raw logs
Gradle Tests - JDK 11 Linux Build Test failures Logs Raw logs
Gradle Tests - JDK 11 Windows Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ Gradle Tests - JDK 11 Linux #

📦 integration-tests/gradle

io.quarkus.gradle.BeanInTestSourcesTest.testBasicMultiModuleBuild() line 15 - More details - Source on GitHub

io.quarkus.gradle.KotlinGRPCProjectBuildTest.testBasicMultiModuleBuild() line 16 - More details - Source on GitHub

io.quarkus.gradle.MultiModuleKotlinProjectBuildTest.testBasicMultiModuleBuild() line 15 - More details - Source on GitHub

io.quarkus.gradle.MultiSourceProjectTest.shouldRunTest() line 16 - More details - Source on GitHub

io.quarkus.gradle.devmode.MultiModuleKotlinProjectDevModeTest.main() line 22 - More details - Source on GitHub


⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

io.quarkus.gradle.BeanInTestSourcesTest.testBasicMultiModuleBuild() line 15 - More details - Source on GitHub

io.quarkus.gradle.KotlinGRPCProjectBuildTest.testBasicMultiModuleBuild() line 16 - More details - Source on GitHub

io.quarkus.gradle.MultiModuleKotlinProjectBuildTest.testBasicMultiModuleBuild() line 15 - More details - Source on GitHub

io.quarkus.gradle.MultiSourceProjectTest.shouldRunTest() line 16 - More details - Source on GitHub

io.quarkus.gradle.devmode.MultiModuleKotlinProjectDevModeTest.main() line 22 - More details - Source on GitHub


⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/resteasy-reactive-rest-client

io.quarkus.it.rest.client.BasicTest.shouldWork line 30 - More details - Source on GitHub

@gastaldi gastaldi force-pushed the resource_merge_item branch from cc2d7a9 to 3f07985 Compare May 10, 2021 20:23
@gastaldi gastaldi dismissed gsmet’s stale review May 11, 2021 02:00

Changes addressed

@gastaldi gastaldi merged commit 02c6f77 into quarkusio:main May 11, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 11, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone May 11, 2021
@gastaldi gastaldi deleted the resource_merge_item branch May 11, 2021 02:00
@gsmet gsmet modified the milestones: 2.0 - main, 1.13.4.Final May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shade-like transformers for uberjar creation
3 participants