dependabot-core icon indicating copy to clipboard operation
dependabot-core copied to clipboard

Added `Mode::EXECUTABLE` flag to `gradlew` files

Open gmazzo opened this issue 3 weeks ago • 1 comments

What are you trying to accomplish?

After #13579, we realised that the executable mode of gradlew was lost when dependabot updates the Gradle Wrapper binaries:

/Users/runner/work/_temp/9c8c4b16-87e9-4ad9-aa50-8fd0b4f2ccd7.sh: line 1: ./gradlew: Permission denied

https://github.com/gmazzo/gradle-codeowners-plugin/actions/runs/19988292200/job/57325370740?pr=251#step:4:21

Smoke tests PR: https://github.com/gmazzo/gradle-codeowners-plugin/pull/251/files

Anything you want to highlight for special attention from reviewers?

I'm assuming that setting file.mode = Mode.EXECUTABLE will be propagated to the PR creation, but needs validation from mantainers.

How will you know you've accomplished your goal?

Since I can't run (or don't know how) to run dependabot locally to open real PRs, I rely on smoke tests to validate the change.

The run with dependabot test reveals that now a new mode: 100755 is added into the output YAML: image

Checklist

  • [x] I have run the complete test suite to ensure all tests and linters pass.
  • [x] I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • [x] I have written clear and descriptive commit messages.
  • [x] I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • [x] I have ensured that the code is well-documented and easy to understand.

gmazzo avatar Dec 06 '25 14:12 gmazzo

@kbukum1 Could you please review this?

yeikel avatar Dec 09 '25 17:12 yeikel

@gmazzo , sorry for late review. I just saw that in the smoke test the mode is getting updated and accordingly content is getting removed. When we update gradle wrapper and create PR, don't we need also gradlew content as well in the PR or we are not sending update for that?

kbukum1 avatar Dec 15 '25 21:12 kbukum1

Hi @kbukum1!

accordingly content is getting removed.

You mean in the PR diff?

The content of the gradlew was updating already with the previous changes. The issue I spotted now is that the +x flag gests removed with the file update in the PR.

Assuming you are asking about the diff of the smoke tests, the + for the mode: "100755" is for the change in the file attribute that this PR meants to fix. It's true that there is a - content: "@rem\r\n@rem Copyright 2015 the original author or right after that, but if you take a closer look, that - does not stands for a diff removing, but rather for a YAML entry.

gmazzo avatar Dec 16 '25 00:12 gmazzo

Hi @kbukum1!

accordingly content is getting removed.

You mean in the PR diff?

The content of the gradlew was updating already with the previous changes. The issue I spotted now is that the +x flag gests removed with the file update in the PR.

Assuming you are asking about the diff of the smoke tests, the + for the mode: "100755" is for the change in the file attribute that this PR meants to fix. It's true that there is a - content: "@rem\r\n@rem Copyright 2015 the original author or right after that, but if you take a closer look, that - does not stands for a diff removing, but rather for a YAML entry.

Thanks for the reply. I am going to deploy this one with the smoke test then. Let me know if you see any issue.

  • https://github.com/dependabot/smoke-tests/pull/353

kbukum1 avatar Dec 16 '25 19:12 kbukum1

  • Updated gradle-wrapper tests repo and hash smoke-tests#353

@gmazzo , can you check the smoke test. There are some issues with smoke test? I am rerunning it but seems like it will fail?

kbukum1 avatar Dec 16 '25 21:12 kbukum1

  • Updated gradle-wrapper tests repo and hash smoke-tests#353

@gmazzo , can you check the smoke test. There are some issues with smoke test? I am rerunning it but seems like it will fail?

I think it was temporary issue. Now it is passed. Merged smoke test as well.

kbukum1 avatar Dec 16 '25 21:12 kbukum1

Thank you both for your support. We're closer and closer to GA 😀

yeikel avatar Dec 16 '25 21:12 yeikel

@kbukum1 was it deployed right? I don't think it worked.

I re-runned the PR with @dependabot recreate, and confirmed it used a new image ghcr.io/dependabot/dependabot-updater-gradle:ef68611130da716d4030d17d24042f85bde8e461 (created at 2025-12-16T20:51:14.997328779Z), but still getting the error: ./gradlew: Permission denied

Can you verify with the Eng team if the Mode attribute is supported by the updater proxy internally? I don't think it's getting propagated correcly into the PR

gmazzo avatar Dec 17 '25 00:12 gmazzo

Ok, apparenly the issue is when "updaing the PR" but, creaing it from scrach does work as expected! Finally: https://github.com/gmazzo/gradle-codeowners-plugin/pull/257

Can you verify with the Eng team if the Mode attribute is supported by the updater proxy internally?

@kbukum1 you may wan't to double check the "@dependabot recreate" flow with the team, but I think we are ready to start rolling this out. At least I don't have any pending thinkg to test

gmazzo avatar Dec 20 '25 21:12 gmazzo

It seems to be working well

https://github.com/yeikel/kafka-ui/pull/281

But I'll do more tests and confirm

yeikel avatar Dec 21 '25 02:12 yeikel