Skip to content

[build-ml-whl] Improve safety and logging#61

Merged
dbarbuzzi merged 4 commits intomainfrom
improve-build-script-safety
Aug 7, 2025
Merged

[build-ml-whl] Improve safety and logging#61
dbarbuzzi merged 4 commits intomainfrom
improve-build-script-safety

Conversation

@dbarbuzzi
Copy link
Contributor

Summary:

Improve general scripting safety and logging of build-ml-whl.

Details:

Recently, the build job started failing: https://github.com/neuralmagic/llm-compressor-testing/actions/runs/16765257143/job/47524176286

/home/runner/_work/_temp/50ab11ad-06e5-463a-aca3-98222a2695e3.sh: line 17: cd: too many arguments

With the logging added in this branch’s initial commit, it was learned that this was due to the project adding another Makefile in a subdirectory, thus confusing the results which only ever expected to find a single Makefile:

makefile_path=./llm-compressor/Makefile
./llm-compressor/docs/Makefile
makefile_dir=./llm-compressor/Makefile
./llm-compressor/docs
/home/runner/_work/_temp/61bfa8a4-a12b-4e4d-8f2d-95bfd16d2ea0.sh: line 24: cd: $'./llm-compressor/Makefile\n./llm-compressor/docs': No such file or directory

The newly added -maxdepth 2 flag for this command limits the depths that it will search for the Makefile in a way that is compatible with our existing projects/usages and reduces the chances this particular issue will occur again. A further future-proofing would be to replace the find usage with an input specifying the dir to run the make command, or at least have it optional so the find command is a fallback if not specified.

Test Plan:

Copy link
Member

@dhuangnm dhuangnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it!

@dbarbuzzi dbarbuzzi added this pull request to the merge queue Aug 7, 2025
Merged via the queue into main with commit 1db28bd Aug 7, 2025
1 check passed
@dbarbuzzi dbarbuzzi deleted the improve-build-script-safety branch August 7, 2025 12:56
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