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

Forward null when parent cannot be found #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aaron-skydio
Copy link

Currently if I run a workflow where the parent commit cannot be found in
the destination (with merge_import from
here
off), I get an exception:

java.util.NoSuchElementException: No value present
        at java.base/java.util.Optional.get(Optional.java:148)
        at com.google.copybara.WorkflowMode$3.run(WorkflowMode.java:261)
        at com.google.copybara.Workflow.run(Workflow.java:288)
        at com.google.copybara.MigrateCmd.run(MigrateCmd.java:92)
        at com.google.copybara.MigrateCmd.run(MigrateCmd.java:69)
        at com.google.copybara.Main.runInternal(Main.java:241)
        at com.google.copybara.Main.run(Main.java:123)
        at com.google.copybara.Main.main(Main.java:102)
ERROR: Unexpected error (please file a bug against copybara): No value present (java.util.NoSuchElementException: No value present)

I think the right thing to do here is convert the empty Optional to
null? In which case I a legitimate error message instead of a crash:

ERROR: Cannot find matching parent commit in the destination. Use '--change-request-parent' flag to force a parent commit to use as baseline in the destination.

Definitely not 100% sure this fix is correct though, this is as much a
bug report as it is a proposed fix.

Topic: copybara-null-baseline

@aaron-skydio
Copy link
Author

Reviews in this chain:
#228 Forward null when parent cannot be found

@aaron-skydio
Copy link
Author

aaron-skydio commented Mar 11, 2023

# head base diff date summary
0 4170f792 f7d77144 diff Mar 10 18:24 PM 1 file changed, 1 insertion(+), 1 deletion(-)
1 f940dc4e c7a43cb0 diff Mar 13 21:18 PM 0 files changed

@aaron-skydio
Copy link
Author

aaron-skydio commented Mar 11, 2023

About the conflict - I see on main (as of 3af038c) this now doesn't crash, but prints a different error message. It seems like this shouldn't recommend passing --baseline-for-merge-import if merge_import isn't enabled?

@aaron-skydio
Copy link
Author

Updated to the actual proposed change to main

Currently if I run a workflow where the parent commit cannot be found in
the destination (with merge_import from
[here](google@c54a5b9)
off), I get an exception:
```
java.util.NoSuchElementException: No value present
        at java.base/java.util.Optional.get(Optional.java:148)
        at com.google.copybara.WorkflowMode$3.run(WorkflowMode.java:261)
        at com.google.copybara.Workflow.run(Workflow.java:288)
        at com.google.copybara.MigrateCmd.run(MigrateCmd.java:92)
        at com.google.copybara.MigrateCmd.run(MigrateCmd.java:69)
        at com.google.copybara.Main.runInternal(Main.java:241)
        at com.google.copybara.Main.run(Main.java:123)
        at com.google.copybara.Main.main(Main.java:102)
ERROR: Unexpected error (please file a bug against copybara): No value present (java.util.NoSuchElementException: No value present)
```

I think the right thing to do here is convert the empty `Optional` to
`null`?  In which case I a legitimate error message instead of a crash:

```
ERROR: Cannot find matching parent commit in the destination. Use '--change-request-parent' flag to force a parent commit to use as baseline in the destination.
```

Definitely not 100% sure this fix is correct though, this is as much a
bug report as it is a proposed fix.

Topic: copybara-null-baseline
@aaron-skydio aaron-skydio force-pushed the aaron/revup/main/copybara-null-baseline branch from f940dc4 to cc69460 Compare April 13, 2024 00:32
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.

2 participants