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

pgpverify:check doesn't honor mvn --offline option #245

Closed
sewe opened this issue Mar 11, 2021 · 5 comments · Fixed by #249
Closed

pgpverify:check doesn't honor mvn --offline option #245

sewe opened this issue Mar 11, 2021 · 5 comments · Fixed by #249
Assignees
Labels
bug Something isn't working.

Comments

@sewe
Copy link

sewe commented Mar 11, 2021

As of version 1.11.0, the pgpverify:check goal always contacts remote keyservers, even if Maven is invoked with the --offline command line option.

IMHO, the goal should simply skip updating its cache and possibly print out a INFO-level message that it did so.

@slawekjaranowski
Copy link
Member

What should be in offline mode done if key doesn't exist in local cache?

@slawekjaranowski slawekjaranowski added the bug Something isn't working. label Mar 11, 2021
@slawekjaranowski
Copy link
Member

I mark it as a bug ... maven offline mode should be respected by design.

@sewe
Copy link
Author

sewe commented Mar 11, 2021

What should be in offline mode done if key doesn't exist in local cache?

I would simply fail the build with an error, similar to when Maven needs to download a dependency that is not yet cached in the local repository.

@joschi
Copy link
Contributor

joschi commented Mar 17, 2021

I would simply fail the build with an error, similar to when Maven needs to download a dependency that is not yet cached in the local repository.

I agree, but then it should be possible to fetch all the required signatures with a separate goal, similar to dependency:go-offline.

@slawekjaranowski
Copy link
Member

for go-offline feature please create new issue.

@slawekjaranowski slawekjaranowski self-assigned this Mar 17, 2021
pzygielo pushed a commit to pzygielo/pgpverify-maven-plugin that referenced this issue May 31, 2024
In the mean time turns out we have two important "implementation details" to obey, and behave correctly:

We need ONE transport to happen (go thru pipe fully) before we can pour the rest onto transport

The HTTP transporter is equipped with "shared" caches for auth that is heavily used by HttpClient to make decisions (ie. about re-attempting request). Current code is quite complicated, but auth sharing happens when one transfer task is done, and that step "inseminates" the cache to be used by others (and skip the auth dance from that point onwards). Similar step happens by transfer by issuing OPTIONS to remote to discover does it deals with WebDAV server or not (is MKCOL dance needed or not). All these states are once task is done shared and remembered for the lifetime of session (per remote repository). Hence, to obey these rules, simple fix is applied: FIRST transfer is sent thru pipe alone fully, and once is done, the REST is pushed in parallel, as at that point, all the auth caches and DAV state of remote repositry are already discovered by first one. This simple "trick" should not introduce big performance loss for parallel vs sequential deploy. This is clearly a band aid for transport-http (did not test with other), but IMHO is a good one, it does the job without sacrificing too much. We will need to refactor transport-http (or any other new transport) to address this issue in future.

We must ensure proper ordering, that was not ensured by origina implementation

Clients when going for metadata are getting them as G -> A -> V (the longest chain in case of snapshot maven plugin). Hence, we must ensure we deploy the opposite order, and we start deploying next group ONLY when we are fully done with previous group. Not ensuring this would leave a window of opportunity of failed build, where for example A level md is present, but V level not yet.

---

https://issues.apache.org/jira/browse/MRESOLVER-319
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Development

Successfully merging a pull request may close this issue.

3 participants