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

Version 4.0.5 fails on a missing dependency #383

Closed
dstepanov opened this issue Jun 20, 2022 · 16 comments · Fixed by #395
Closed

Version 4.0.5 fails on a missing dependency #383

dstepanov opened this issue Jun 20, 2022 · 16 comments · Fixed by #395

Comments

@dstepanov
Copy link
Contributor

It looks like org.jboss.test-audit:jboss-test-audit-impl is not available in the maven central:

Could not find org.jboss.test-audit:jboss-test-audit-impl:2.0.0.Final.
     Searched in the following locations:
       - https://repo.maven.apache.org/maven2/org/jboss/test-audit/jboss-test-audit-impl/2.0.0.Final/jboss-test-audit-impl-2.0.0.Final.pom

It is available in the Jboss repo but it would be nice not to require the use of a custom repo.

@manovotn
Copy link
Contributor

Yea, I think this one is not synced with Central.

I am not sure if any other project still uses this, I recall updating it for the sake of EE 9 but we should probably look into what it actually does for this projects (it was a test coverage tool IIRC and probably required update of the TCK audit files as well?) and move it as a submodule here.

@manovotn
Copy link
Contributor

BTW this is not just 4.0.5, this should be true for any CDI TCK version.

@starksm64
Copy link
Contributor

starksm64 commented Jun 20, 2022

The test-audit dependency is only used during the build of the TCK dist. It can be disable in one of two ways:

  1. When importing a TCK dependency exclude the two test-audit dependencies:
    <dependency>
      <groupId>jakarta.enterprise</groupId>
      <artifactId>cdi-tck-core-impl</artifactId>
      <version>${cdi.tck.version}</version>
      <scope>test</scope>
      <exclusions>
        <exclusion>
          <groupId>org.jboss.test-audit</groupId>
          <artifactId>jboss-test-audit-api</artifactId>
        </exclusion>
        <exclusion>
          <groupId>org.jboss.test-audit</groupId>
          <artifactId>jboss-test-audit-impl</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
  1. By disabling the the tck-audit profile by specifying -DskipTckAudit on the command line when running.

The tck-audit profile is enabled by default because you cannot build the TCK without it enabled. We could disable it by default and have to enable it during builds.

The other thing I have been meaning to look into is to generate a separate release pom for the TCK artifacts that do not include these build related profiles.

@manovotn
Copy link
Contributor

manovotn commented Jun 21, 2022

This should now be resolved via #388

@starksm64
Copy link
Contributor

Going forward, starting when there is a 4.0.6 release, we won't be leaking test-audit dependency that is only need for building the TCK to the released test artifacts.

@manovotn
Copy link
Contributor

Coming back to this, I don't think this was a good decision. Not being able to build the project without a switch smells like bad practice. I'd rather people excluded the dependency if it is a problem.
Cc @Ladicek, maybe he'll have some other idea how to solve this?

@Ladicek
Copy link
Contributor

Ladicek commented Jun 29, 2022

My understanding is that you can build the project without an extra cmdline argument, it just won't generate the audit protocols?

@manovotn
Copy link
Contributor

@Ladicek that was my understanding as well (and shame on me for not trying it out) but that's not the case.
You get compilation errors because of all the annotations such as @SpecAssertion or @SpecVersion

@Ladicek
Copy link
Contributor

Ladicek commented Jun 29, 2022

Ouch! Yea I didn't try it either :-) OK, that's bad. Frankly, I don't understand why we don't publish jboss-test-audit-impl into Maven Central, because jboss-test-audit-api is there (and so is jboss-test-audit-parent). If we published -impl to Central, the extra profile and everything would become completely obsolete.

@manovotn
Copy link
Contributor

I agree.
I've created a JBoss nexus JIRA to ask if we could have impl artifact published as well - https://issues.redhat.com/browse/NEXUS-609
Meanwhile, I will reopen this issue to keep track of it.

@manovotn manovotn reopened this Jun 29, 2022
@starksm64
Copy link
Contributor

Yes, the latest workaround the jboss repo 503 errors was focused on the runtime side of usage, not the build side. There still is the issue of having dependencies in the tck artifacts that are not used. The tck-audit*.xml files and annotations generation should probably be in a separate project that is consumed by the tck test artifacts. There is no reason to build these every time the tests are built.

@manovotn
Copy link
Contributor

Yes, the latest workaround the jboss repo 503 errors was focused on the runtime side of usage, not the build side.

I am not sure I know what are you talking about? This impl artifact has never had a single version published into central which seems like a sort of deliberate exclusion?

There is no reason to build these every time the tests are built.

There was always the option to do -DskipTckAudit if you want to. I am just of the opinion that a maven project should be buildable by plain mvn clean install :)

There still is the issue of having dependencies in the tck artifacts that are not used.

True, a lot of bigger projects import dependencies with exlusions to avoid this. However, a project that imports TCKs is likely a projects that's meant to test run it in which case I don't see any big harm in just having it there. Unless I am missing something.

@starksm64
Copy link
Contributor

One could use -DskipTckAudit, but every downstream consumer who was running into 503 errors had to do that. Since we build the tck artifacts, I switched the burden of avoiding the jboss repo errors to the build step in the eclipse ci environment so that every downstream user did not have to deal with the problem.

It was projects that were importing the TCKs to run them that were seeing the unused test-audit-impl dependency causing errors when the tck-audit profile was enabled by default.

@manovotn
Copy link
Contributor

I understand, but if we manage to get the impl artifact to Maven Central, these issues should just disappear.

@starksm64
Copy link
Contributor

Correct, but there still is the issue of building the spec annotations every time, for both the impl and web tck artifacts.

@manovotn
Copy link
Contributor

@starksm64 @dstepanov got an update from the JIRA I created, the org.jboss.test-audit:jboss-test-audit-impl artifact should now be available in Central as well.
I can even see all the versions in there - https://repo1.maven.org/maven2/org/jboss/test-audit/jboss-test-audit-impl/

With that, the TCK build should no longer be an issue even in the default setup. Therefore, I'll revert changes in #388

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants