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

Action must fail when options can't be honored due to downloading from API #1879

Open
loganmzz opened this issue Sep 6, 2024 · 4 comments
Open

Comments

@loganmzz
Copy link

loganmzz commented Sep 6, 2024

It seems action can fallback to API download if Git client is missing.

However:

  • it means .git directory is missing, and it's not a Git repository
  • Such no Git operation are permitted (whatever using CLI client or embedded client libriaries)

Possible invalid options in such case:

  • filter
  • sparse-checkout
  • sparse-checkout-cone-mode
  • fetch-depth
  • fetch-tags
  • submodules
@loganmzz loganmzz changed the title Action must fail when options can be honored due to downloading from API Action must fail when options can't be honored due to downloading from API Sep 6, 2024
@jekru
Copy link

jekru commented Sep 26, 2024

Letting the action fail might be a bit harsh for backwards compatibility reasons, how about a warning message?

# The repository will be downloaded using the GitHub REST API
# To create a local Git repository instead, add Git 2.18 or higher to the PATH
! WARNING: Some options are not available when using the Github REST API

@loganmzz
Copy link
Author

  1. No one is reading logs until errors
  2. It will just crash later on when you want to use Git or may have strange/unpredictable behavior if trying and just ignoring in case of failure.

@jekru
Copy link

jekru commented Sep 26, 2024

No one is reading logs until errors

I think it's quite common to check logs when creating or adjusting a workflow. However, once a workflow is up and running, I agree.

It will just crash later on when you want to use Git or may have strange/unpredictable behavior if trying and just ignoring in case of failure.

Let's assume this really would lead to crashes in most cases, how many users are relying on the current behavior because they build workarounds or adjusted their workflows accodingly? My feeling is that such a breaking change is too risky and may even lead to a net negative impact in the short/mid term.

Some ideas for further discussion:

  • Release in upcoming major version
  • Introduce boolean field exit_on_mismatch to on/off the proposed functionality
  • Add warning message

@loganmzz
Copy link
Author

adding on exit flag defaulting to off sounds good to me ! This is the best trade-off.

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

No branches or pull requests

2 participants