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

Update mvnd to include Maven Resolver 1.7 #507

Merged
merged 1 commit into from Nov 26, 2021
Merged

Update mvnd to include Maven Resolver 1.7 #507

merged 1 commit into from Nov 26, 2021

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Oct 20, 2021

As mvnd itself is Java8 lang level, it is completely
okay to up resolver to 1.7 (that is java8 as well)
and get it's improvements.

The only reason why Maven 3.8.x CANNOT use resolver
1.7 is that it is still Java7 level.

@michael-o
Copy link
Member

Makes sense!

@cstamas
Copy link
Member Author

cstamas commented Oct 21, 2021

Unsure about macOS failure (am on Linux), is it me or something else?

@gnodet
Copy link
Contributor

gnodet commented Oct 21, 2021

Unsure about macOS failure (am on Linux), is it me or something else?

The whole build did pass for me, let me start CI again.

@cstamas
Copy link
Member Author

cstamas commented Oct 21, 2021

So, with 1.7.x you get "local" (semaphore or reentrantrw) sync context out of box, but also 1.7.3 may have this one as well: apache/maven-resolver#131

@gnodet
Copy link
Contributor

gnodet commented Oct 21, 2021

So, with 1.7.x you get "local" (semaphore or reentrantrw) sync context out of box, but also 1.7.3 may have this one as well: apache/maven-resolver#131

So +1 for the local one, though it has been already added to mvnd.
I still don't really understand the benefits of the NamedLock : this interface seems a regression to me compared to the SyncContext ... As for the implementation, my understanding is that they require an external process to be setup, which I don't think is worth it in simple cases, which means it's a good thing when your local repository is shared across several servers (is that ever the case ?). For simple scenario, I'd much rather use the mvnd IPC implementation at https://github.com/mvndaemon/mvnd/tree/master/sync/src/main/java/org/mvndaemon/mvnd/sync which requires zero setup and should be faster (using unix sockets and a native server when possible).

@cstamas
Copy link
Member Author

cstamas commented Oct 21, 2021

No, that's misunderstanding. There are several NamedLock providers and only SOME of them needs extra infra (external process):

  • rwlock-local (default), uses JVM ReentrantReadWriteLock per lock name, usable for MT builds.
  • semaphore-local, uses JVM Semaphore per lock name, usable for MT builds.
  • noop, implement no-op locking (no locking). For experimenting only. Has same functionality as old "nolock"
    SyncContextFactory implementation.

and the pointed PR adds file-lock provider using NIO FileLock usable for MP. None of these above needs anything external (infra/process).

There are TWO MP providers, for Redis and Hazelcast, and those two DO need external Redis or Hazelcast.

@cstamas
Copy link
Member Author

cstamas commented Oct 21, 2021

Also, NamedLock does not get in your way, you don't have to use them (as mvnd does: it replaces SyncContextFactory), they are merely an "internal detail", how resolver locks are implemented and used via resolver "adapter" SyncContextFactory.

@cstamas cstamas mentioned this pull request Oct 21, 2021
@gnodet
Copy link
Contributor

gnodet commented Nov 3, 2021

No, that's misunderstanding. There are several NamedLock providers and only SOME of them needs extra infra (external process):

  • rwlock-local (default), uses JVM ReentrantReadWriteLock per lock name, usable for MT builds.
  • semaphore-local, uses JVM Semaphore per lock name, usable for MT builds.
  • noop, implement no-op locking (no locking). For experimenting only. Has same functionality as old "nolock"
    SyncContextFactory implementation.

and the pointed PR adds file-lock provider using NIO FileLock usable for MP. None of these above needs anything external (infra/process).

There are TWO MP providers, for Redis and Hazelcast, and those two DO need external Redis or Hazelcast.

Yes, what I meant is that in the context of mvnd, those two are the only ones that can provide locking between the daemons. The file lock seems really promising though.

@ppalaga
Copy link
Contributor

ppalaga commented Nov 11, 2021

@gnodet is this good to merge?
@cstamas could you please squash your commits and possibly rebase on top of recent main?

As mvnd itself is Java8 lang level, it is completely
okay to up resolver to 1.7 (that is java8 as well).
The only reason why Maven 3.8.x CANNOT use resolver
1.7 is that it is still Java7 level.
@cstamas
Copy link
Member Author

cstamas commented Nov 12, 2021

@ppalaga done

@gnodet gnodet merged commit 807409d into apache:master Nov 26, 2021
@cstamas cstamas deleted the maven-resolver-17 branch November 29, 2021 19:17
@gnodet gnodet added this to the 0.7.1 milestone Dec 7, 2021
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 this pull request may close these issues.

None yet

4 participants