feat: add CI check for executable permissions on shell scripts#62
Merged
Conversation
Add a new Makefile target and CI workflow job to verify that all shell scripts in the ci/ directory have executable permissions. This prevents deployment issues where scripts fail to execute due to missing permissions. Changes: - Added check-ci-scripts-executable target to Makefile.jinja - Added ci-scripts-permissions job to continuous-integration.yml workflow - Check validates all .sh and .sh.jinja files in ci/ directory - Provides clear error message with fix command when permissions are missing
Move the executable permissions check logic from the Makefile into a dedicated script (ci/check-executable-permissions.sh) for better maintainability and testability. Also remove the Nix dependency from the CI workflow since this check only requires basic bash/file operations. Changes: - Created ci/check-executable-permissions.sh with the check logic - Simplified Makefile target to just call the script - Removed Nix setup from ci-scripts-permissions job - No dependencies required - runs directly with make Benefits: - Faster CI execution (no Nix setup overhead) - Script can be run independently for testing - Consistent with other CI scripts (publish-binary.sh, etc.)
Rename all components to use consistent "scripts-permissions" naming: - Script: check-executable-permissions.sh → check-scripts-permissions.sh - Makefile target: check-ci-scripts-executable → check-scripts-permissions - Workflow job: ci-scripts-permissions → scripts-permissions - Step name: simplified to "Check scripts permissions" This creates a more consistent and concise naming pattern across the codebase.
Match the style and conventions used by other CI scripts: - Use #!/usr/bin/env sh instead of bash - Use long-form flags: set -o errexit, set -o xtrace - Remove informational echo statements (silent success) - Keep only error messages for failures - Use tabs for indentation - Set file permissions to 755 (rwxr-xr-x) This ensures consistency across all CI scripts and reduces noise in output.
Simplify the permissions check script: - Remove all echo statements (silent output, only xtrace) - Use clearer variable name: exit_code instead of failed - Only check .sh files, skip .sh.jinja files - Exit directly with the exit code The script now relies on set -o xtrace for debugging output and exits silently with code 0 on success or 1 on failure.
DeveloperC286
force-pushed
the
claude/check-rust-ci-permissions-N7fhf
branch
from
ac94148 to
9a93816
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a new Makefile target and CI workflow job to verify that all shell
scripts in the ci/ directory have executable permissions. This prevents
deployment issues where scripts fail to execute due to missing permissions.
Changes: