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

fix: Update Maven wrapper and document its use to fix version resolution #7620

Merged
merged 1 commit into from
Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .mvn/wrapper/maven-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# custmized maven that help avoid download only highest poms from version range
# https://issues.apache.org/jira/browse/MRESOLVER-164
distributionUrl=https://confluent-packaging-tools.s3-us-west-2.amazonaws.com/apache-maven-3.6.3.1-bin.zip
distributionUrl=https://confluent-packaging-tools.s3-us-west-2.amazonaws.com/apache-maven-3.8.1.2-bin.zip
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the version @xli1996 kindly built after merging confluentinc/maven#1.

wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar
47 changes: 46 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,47 @@ If you have any questions about how to contribute, either [create a GH issue](ht

## Developing KSQL

### About the Apache Maven wrapper

Development versions of KSQL use version ranges for dependencies
on other Confluent projects, and due to
[a bug in Apache Maven](https://issues.apache.org/jira/browse/MRESOLVER-164),
you may find that both Maven and your IDE download hundreds or thousands
of pom files while resolving dependencies. They get cached, so it will
be most apparent on fresh forks or switching to branches that you haven't
used in a while.

Until we are able to get a fix in and released officially, we need to work
around the bug. We have added a Maven wrapper script and configured
it to download a patched version of Maven. You can use this wrapper simply by
substituting `./mvnw` instead of `mvn` for every Maven invocation.
The patch is experimental, so if it causes you some trouble, you can switch
back to the official release just by using `mvn` again. Please let us know
if you do have any trouble with the wrapper.
Comment on lines +12 to +26
Copy link
Member Author

Choose a reason for hiding this comment

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

I felt like some kind of context would be really nice for newcomers to the codebase.


You can also configure IntelliJ IDEA to use the patched version of Maven.
Copy link
Member Author

Choose a reason for hiding this comment

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

This will probably be welcome news for devs. Hopefully, it works as expected! I have been having a couple of odd problems today, but I think it's just because master is somewhat broken right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice~

First, get the path to the patched Maven home:

```shell
$ ./mvnw -v
...
Maven home: /home/john/.m2/wrapper/dists/apache-maven-3.8.1.2-bin/1npbma9t0n1k5b22fpopvupbmn/apache-maven-3.8.1.2
...
```

Then, update your IDEA Maven home in **Settings > Build, Execution, Deployment > Build Tools > Maven**
and set the value of **Maven home path** to the same directory listed as the "Maven home" above.
In the above example, it is `/home/john/.m2/wrapper/dists/apache-maven-3.8.1.2-bin/1npbma9t0n1k5b22fpopvupbmn/apache-maven-3.8.1.2`,
but it may be different on your machine.

There is likely a similar option available for other IDEs.

### Building and running KSQL locally

To build and run KSQL locally, run the following commands:

```shell
$ ./mvnw clean package -DskipTests -DversionFilter
$ ./mvnw clean package -DskipTests
Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessary anymore!

$ ./bin/ksql-server-start -daemon config/ksql-server.properties
$ ./bin/ksql
```
Expand All @@ -24,6 +59,16 @@ including any errors.
If you would rather have the KSQL server logs spool to the console, then
drop the `-daemon` switch, and start the CLI in a second console.

#### Running in an IDE

You can create a run configuration in your IDE for the main class:
`io.confluent.ksql.rest.server.KsqlServerMain`, using the classpath
of the `ksqldb-rest-app` module, and specifying the path to a config
file as the program argument. There is a basic config available in
`config/ksql-server.properties`.

You may wish to be sure you have already configured your IDE to use
the Maven wrapper (see "About the Apache Maven wrapper", above).
Comment on lines +62 to +71
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated, but I felt it was a bummer to have to read through ksql-server-start to figure out which class to run in IDEA.


### Testing changes locally

Expand Down