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

com.google.flogger:flogger:0.5.1 pom.xml is referencing itself #152

Closed
MosheElisha opened this issue Apr 7, 2020 · 14 comments
Closed

com.google.flogger:flogger:0.5.1 pom.xml is referencing itself #152

MosheElisha opened this issue Apr 7, 2020 · 14 comments
Assignees
Labels
P2 type=defect Bug, not working as expected

Comments

@MosheElisha
Copy link

Hi,

When running maven site plugin I get a warning:

[INFO] --- maven-site-plugin:3.9.0:site (default-site) @ common ---
[INFO] configuring report plugin org.apache.maven.plugins:maven-project-info-reports-plugin:3.0.0
[INFO] 15 reports detected for maven-project-info-reports-plugin:3.0.0: ci-management, dependencies, dependency-info, dependency-management, distribution-management, index, issue-management, licenses, mailing-lists, modules, plugin-management, plugins, scm, summary, team
[INFO] Rendering site with default locale English (en)
[WARNING] No project URL defined - decoration links will not be relativized!
[INFO] Rendering content with org.apache.maven.skins:maven-default-skin:jar:1.3 skin.
[INFO] Generating "Dependency Information" report --- maven-project-info-reports-plugin:3.0.0:dependency-info
[INFO] Generating "Dependency Management" report --- maven-project-info-reports-plugin:3.0.0:dependency-management
[WARNING] Unable to create Maven project for com.google.flogger:flogger:jar:0.5.1 from repository.
org.apache.maven.project.ProjectBuildingException: Some problems were encountered while processing the POMs:
[FATAL] 'dependencies.dependency.[com.google.flogger:flogger:0.5.1]' for com.google.flogger:flogger:0.5.1 is referencing itself. @ line 58, column 13
...

I indeed see the self dependency in the pom.xml at line 58
flogger-0.5.1.pom.txt

This does not prevent the "maven site" from finishing successfully but probably will be good to fix.

@cgdecker cgdecker added P3 type=defect Bug, not working as expected labels Apr 8, 2020
@veblush
Copy link

veblush commented May 14, 2020

Can you please bump this priority to P1 or P2 since even the latest version of copy_dependency fails to copy all dependencies because of this error.

Error message:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:copy-dependencies (default-cli) on project xyz: Some problems were encountered while processing the POMs:
[ERROR] [FATAL] 'dependencies.dependency.[com.google.flogger:flogger:0.5.1]' for com.google.flogger:flogger:0.5.1 is referencing itself. @ line 58, column 13: 1 problem was encountered while building the effective model for com.google.flogger:flogger:0.5.1
[ERROR] [FATAL] 'dependencies.dependency.[com.google.flogger:flogger:0.5.1]' for com.google.flogger:flogger:0.5.1 is referencing itself. @ line 58, column 13

@cpovirk
Copy link
Member

cpovirk commented May 14, 2020

I think we autogenerate this pom.xml with the same machinery as we use for Dagger. I wonder why it's not affected when Flogger is?

@cpovirk
Copy link
Member

cpovirk commented May 14, 2020

Maybe I can binary-search my way out of this, given that 0.5 is affected and 0.5.1 is not.

No promises, but I will see if I can figure it out quickly.

Thank you for the report.

@cpovirk
Copy link
Member

cpovirk commented May 14, 2020

There are very few commits between the releases, and 4b78b14 looked suspicious, so I didn't even need git bisect.

I suspect that the problem is the api_pom -> log_sites -> api chain. I'm not sure what the best thing to do about that is.

@cpovirk
Copy link
Member

cpovirk commented May 14, 2020

Internally, I just speculated:

Maybe all of the libraries in pom_file.targets should be exported from a single target that sets maven_coordinates? This hypothetical new target would be much like :api is today.

@suztomo
Copy link
Member

suztomo commented Jun 1, 2020

I think this invalid pom.xml hit Apache Beam's updateOfflineRepository task (BEAM-10157).

@cpovirk Do you know the root cause now? I'm thinking to start troubleshooting if it's not yet done. My memo: https://gist.github.com/suztomo/fff07dc7bcdb5a7d4647e86b32050882

suztomo added a commit to suztomo/flogger that referenced this issue Jun 1, 2020
@netdpb netdpb added P2 and removed P3 labels Jun 2, 2020
@cpovirk
Copy link
Member

cpovirk commented Jun 2, 2020

I don't have a deep understanding here, but I have a general theory. I will poke around a little to see what I can find.

@cpovirk cpovirk self-assigned this Jun 2, 2020
@cpovirk
Copy link
Member

cpovirk commented Jun 2, 2020

I see that removing log_sites from pom_file (as in #165) doesn't affect the jar, only the pom.xml. That suggests that it's at least a good workaround.

In the long term, it probably "should" be listed in pom_file. That way, if log_sites someday has a third-party dependency that the rest of Flogger does not, that dependency will show up in the generated pom.xml.

I'm going to see if there's an easy way to do something like #152 (comment). Otherwise, I think we should accept your PR while we figure out a long-term solution.

@cpovirk
Copy link
Member

cpovirk commented Jun 2, 2020

I wasted a little time before concluding that, fundamentally, the problem is the mismatch between our internal target structure and our external artifact structure -- or rather, the fact that our external target structure follows the former rather than the latter. I mailed a proposed fix (as an internal CL).

cpovirk added a commit to cpovirk/flogger that referenced this issue Jun 9, 2020
The problem: While we want the `pom_file` target to depend directly on the target with `maven_coordinates`, we do _not_ want it to depend on _other_ targets that _in turn_ depend on the target with `maven_coordinates`. Prior to this CL, the `pom_file` target depends on `:log_sites`, which in turn depends on `:api`, which has `maven_coordinates`.

This CL changes the setup of our _external_ `BUILD` file. Specifically, it puts _all_ of the main Flogger artifact into a single `:api` target (aside from `:platform_provider`, since we create by directly writing bytecode, rather than compiling Java sources). This makes it impossible for a target to both be included in the `pom_file` _and_ depend on `:api`.

Tested with...
- release/install-local-snapshot.sh && grep project.version $HOME/.m2/repository/com/google/flogger/flogger/LOCAL-SNAPSHOT/flogger-LOCAL-SNAPSHOT.pom (and verified no other changes to the pom)
- diffing of the contents of the jar generated with and without this CL (no diffs)
- comparison of $HOME/.m2/repository/com/google/flogger/flogger-*/LOCAL-SNAPSHOT/flogger-*-LOCAL-SNAPSHOT.pom with and without this CL (no diffs)

Fixes google#152
Closes google#165 (by implementing an alternative)

RELNOTES=Eliminated self-reference in pom.xml.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=314530982
@cpovirk cpovirk closed this as completed in 6a468cc Jun 9, 2020
@cpovirk
Copy link
Member

cpovirk commented Jun 9, 2020

Fixed in master. We'll need to do a release for that to solve the problems you're seeing, of course. I don't remember offhand if I have permission to publish a release. I will try to have a look within the next week or so.

Thanks again for the report.

@suztomo
Copy link
Member

suztomo commented Jun 10, 2020

@cpovirk Thank you.

@MosheElisha
Copy link
Author

@cpovirk Can you please check if you can release a new version with this fix? Thanks.

@Jdban
Copy link

Jdban commented Jan 27, 2021

@cpovirk yeah if we could get a fix that'd be great

@cpovirk
Copy link
Member

cpovirk commented Mar 15, 2021

@cgdecker is planning to do a release in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 type=defect Bug, not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants