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

(Makefile) remove-spaces command does not work on GNU system #4327

Open
damsien opened this issue Nov 15, 2024 · 6 comments
Open

(Makefile) remove-spaces command does not work on GNU system #4327

damsien opened this issue Nov 15, 2024 · 6 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@damsien
Copy link
Contributor

damsien commented Nov 15, 2024

What broke? What's expected?

This issue is related to the Makefile of Kubebuilder and not to the generated Makefile used for the operator.

The command make remove-spaces executed on a GNU system raises an error.

kubebuilder/Makefile

Lines 71 to 74 in e9beffb

.PHONY: remove-spaces
remove-spaces:
@echo "Removing trailing spaces"
@find . -type f -name "*.md" -exec sed -i '' 's/[[:space:]]*$$//' {} + || true

That is due to a difference between the sed of the GNU system and the OS X system (this command works well on MacOS). It is well explained here.

My proposal to solve this is to add a check of the system.

.PHONY: remove-spaces
remove-spaces:
	@echo "Removing trailing spaces"
        @SED_CMD="sed -i ''" && [ "$$(uname)" != "Darwin" ] && SED_CMD="sed -i"; \
	find . -type f -name "*.md" -exec $$SED_CMD 's/[[:space:]]*$$//' {} + || true

Reproducing this issue

No response

KubeBuilder (CLI) Version

4.3.1

PROJECT version

No response

Plugin versions

No response

Other versions

No response

Extra Labels

No response

@damsien damsien added the kind/bug Categorizes issue or PR as related to a bug. label Nov 15, 2024
@camilamacedo86
Copy link
Member

that is working in the CI (which is ubuntu)
but I am fine with any change that works for mac and linux
feel free to open a pr with your suggestion

@damsien
Copy link
Contributor Author

damsien commented Nov 15, 2024

From what I saw in the .github/workflows, there is no call to the make remove-spaces command. This command is only called once by the make generate command.

kubebuilder/Makefile

Lines 66 to 69 in 20a9955

.PHONY: generate
generate: generate-testdata generate-docs ## Update/generate all mock data. You should run this commands to update the mock data after your changes.
go mod tidy
make remove-spaces

Correct me if I am wrong but the make generate command is never executed during the CI process.

PS: the "Trailing spaces" CI job only check for trailing spaces without trying to remove them (make test-spaces).

jobs:
lint:
name: "Check Trailing"
runs-on: ubuntu-latest
# Pull requests from the same repository won't trigger this checks as they were already triggered by the push
if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository)
steps:
- name: Clone the code
uses: actions/checkout@v4
- name: Run check
run: make test-spaces

@damsien
Copy link
Contributor Author

damsien commented Nov 16, 2024

Should we keep this issue open? I wanted to show you the error. In the following screenshot, the first command is executed without the "fix" I made. And the second command is executed with the "fix".

image

The fix I made is wrong. I can try to find a friend that have a mac to actually test the command or wait for someone that want to take the issue.

@camilamacedo86
Copy link
Member

HI @damsien

The way that it is today I know that work on Mac
So, could we not:

If is mac then do as it is today
else
do the way that worked for you in linux

WDYT?
Do you want to try it?
I can test locally the next one.

@damsien
Copy link
Contributor Author

damsien commented Nov 16, 2024

Yes we can do this, I'm not going to create the PR today but I'll work on it

@damsien
Copy link
Contributor Author

damsien commented Nov 16, 2024

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants