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

[1.x] mixup in case of DISPLAY messages type #1037

Merged
merged 2 commits into from
Jun 20, 2024
Merged

[1.x] mixup in case of DISPLAY messages type #1037

merged 2 commits into from
Jun 20, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jun 19, 2024

ProjectEvent message mixup fixed.

Fixes #1036

The event can be ProjectMessage as well.
@cstamas cstamas requested a review from gnodet June 19, 2024 11:35
@cstamas cstamas self-assigned this Jun 19, 2024
@cstamas cstamas changed the title Fixes #1036 ClassCastEx with some events Jun 19, 2024
@cstamas cstamas marked this pull request as ready for review June 19, 2024 11:36
@cstamas
Copy link
Member Author

cstamas commented Jun 19, 2024

Locally verified:
mvnd 1.0.0 does this https://gist.github.com/cstamas/7090de741c6c7df393198d6b7962dc4e
This PR fixed mvnd does successfully executes command from the gist above.

The message that fails with CCEx:

[standalone-pom] Choose archetype:

@cstamas cstamas requested a review from ppalaga June 19, 2024 11:39
@cstamas cstamas linked an issue Jun 19, 2024 that may be closed by this pull request
@cstamas cstamas added this to the 1.0.1 milestone Jun 19, 2024
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

It's wrong imho.
The type of message identifies how it will be read, see

case BUILD_REQUEST:
return BuildRequest.read(input);
case BUILD_STARTED:
return BuildStarted.read(input);
case BUILD_FINISHED:
return BuildFinished.read(input);
case MOJO_STARTED:
return MojoStartedEvent.read(input);
case PROJECT_LOG_MESSAGE:
case DISPLAY:
return ProjectEvent.read(type, input);
case BUILD_EXCEPTION:
return BuildException.read(input);
case KEEP_ALIVE:
return BareMessage.KEEP_ALIVE_SINGLETON;
case STOP:
return BareMessage.STOP_SINGLETON;
case PROMPT:
return Prompt.read(input);
case PROMPT_RESPONSE:
return PromptResponse.read(input);
case PROJECT_STARTED:
case PROJECT_STOPPED:
case BUILD_STATUS:
case BUILD_LOG_MESSAGE:
return StringMessage.read(type, input);
case CANCEL_BUILD:
return BareMessage.CANCEL_BUILD_SINGLETON;
case TRANSFER_INITIATED:
case TRANSFER_STARTED:
case TRANSFER_PROGRESSED:
case TRANSFER_CORRUPTED:
case TRANSFER_SUCCEEDED:
case TRANSFER_FAILED:
return TransferEvent.read(type, input);
case EXECUTION_FAILURE:
return ExecutionFailureEvent.read(input);
case PRINT_OUT:
case PRINT_ERR:
return StringMessage.read(type, input);
case REQUEST_INPUT:
return RequestInput.read(input);
case INPUT_DATA:
return InputData.read(input);

So we can't have two message classes for the same message type.

@cstamas
Copy link
Member Author

cstamas commented Jun 19, 2024

Hm, so there is some mixup, as:

public static StringMessage display(String message) {
return new StringMessage(DISPLAY, message);
}

but there is this as well:

https://github.com/apache/maven-mvnd/blob/mvnd-1.x/common/src/main/java/org/mvndaemon/mvnd/common/Message.java#L1171-L1173

@cstamas cstamas changed the title ClassCastEx with some events [1.x] mixup in case of DISPLAY messages type Jun 20, 2024
@cstamas cstamas merged commit 0e27c24 into mvnd-1.x Jun 20, 2024
5 checks passed
@cstamas cstamas deleted the mvnd-1036 branch June 20, 2024 07:01
@gnodet gnodet mentioned this pull request Nov 25, 2024
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.

mvnd archetype:generate failed
2 participants