[ci] [python-package] re-enable scikit-build-core binary stripping

[jameslamb]

fixes #6609

scikit-build-core can be configured to strip debug symbols out of compiled objects. That's been turned off in LightGBM because it caused problems for pydistcheck (at a time when configuring pydistcheck to only run certain checks was not supported).

The relevant issue in pydistcheck (jameslamb/pydistcheck#235) has been fixed in the recent 0.9.1 release of that tool, so this re-enables stripping.

Notes for Reviewers

This change should not affect lightgbm wheels, as they're already build without debug symbols.

https://github.com/microsoft/LightGBM/blob/6437645c4a0c17046be59e4f57d09952e2e0185f/python-package/pyproject.toml#L74

Enabling automatic stripping in scikit-build-core is just an extra layer of protection against compiler flags like -g unintentionally making it into builds.

[StrikerRUS]

LGTM! Nice change!

[StrikerRUS]

Can you please also try to use the latest gcc version here with new pydistcheck?

https://github.com/microsoft/LightGBM/blob/82c85e4032fa12a23de341888ca545757761682e/.ci/test.sh#L16-L18

Refer to #6608 (comment).

[StrikerRUS]

For failing lint job, I guess we just need to lowercase Exit commands here:
https://github.com/microsoft/LightGBM/blob/82c85e4032fa12a23de341888ca545757761682e/.ci/test-windows.ps1#L21
https://github.com/microsoft/LightGBM/blob/82c85e4032fa12a23de341888ca545757761682e/.ci/test-windows.ps1#L28
https://github.com/microsoft/LightGBM/blob/82c85e4032fa12a23de341888ca545757761682e/.ci/test-windows.ps1#L62

I can commit changes right in this branch or make a PR against ci/strip-binaries branch if you wish.

[jameslamb]

I can commit changes right in this branch or make a PR against ci/strip-binaries branch if you wish.

Thanks @StrikerRUS ! You can push directly to this branch whenever you want.

I should have more time to work on LightGBM again starting tomorrow, so if you don't get to it by then I can handle updating this to get CI unblocked.

[jameslamb]

This is new:

^ @shiyu1994 did some repo setting change? Are we now dismissing reviewers when a new commit is pushed to a PR?

If so, could you please change that? I think it'd slow down development here, and isn't really necessary... we generally trust each other to use good judgment about whether or not to ask for a new review if significant changes are pushed after an approval and every maintainer has the ability to leave a blocking "request changes" review.

I'm guessing it is this "Dismiss stale pull request approvals when new commits are pushed" setting in the branch protection on master.

(that is a screenshot from the settings of one of my personal repos... I don't have permission to see those settings here)

I'd also like to understand why this changed... is it something that someone changed Microsoft-wide? Or something you did?

[jameslamb]

Also... @StrikerRUS I pushed 8ce7095 implementing both of your suggestions:

• fixing the powershell linting stuff
• trying gcc-14 on macOS

Thanks for remembering that conversation about updating to gcc-14, I'd forgotten!

[jameslamb]

All of the Azure DevOps jobs did pass: https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=17701&view=results

But they were not successfully reported back to GitHub.

Going to try pushing a new empty commit.

[jameslamb]

Oh wait! I just realized... there have not been any jobs picked up on Azure DevOps in the last 5 days (!!!)

Look at https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build?definitionId=1

Looks like my recent commit here did not trigger a build there.

[jameslamb]

@shiyu1994 I think we will need your help to figure out why Azure DevOps is not running jobs for new commits pushed to PRs. I don't have sufficient permissions in this repo and Azure DevOps to see all of the settings.

Can you please look into it? You can push new empty commits to this PR's branch to test.

CI has been mostly blocked in this project for several weeks now, I would really like to get it working again as soon as possible, so external contributors' PRs can be merged and we don't lose momentum with them.

[StrikerRUS]

@jameslamb Thanks a lot for the recent improvements!
Reapproving.

[shiyu1994]

Sorry for missing the message and for blocking the commits and PRs. I'll figure it out tomorrow.

[StrikerRUS]

Kindly ping @shiyu1994

[shiyu1994]

/AzurePipelines run

[jameslamb]

@shiyu1994 what is the reason for now requiring this /AzurePipelines run command? If it is a security thing, could it be avoided if a pull request contains only signed commits from known contributors?

Also, how will this work for Azure DevOps builds which are not triggered by pull requests? We have builds running there on merges to master and, more importantly, triggered by a new release being published.

[shiyu1994]

Yes. It is a security thing. I've just changed the setting. Previously the comment was required for all PRs. I believe now we do not need comment for PRs from the team.

[shiyu1994]

Also, how will this work for Azure DevOps builds which are not triggered by pull requests? We have builds running there on merges to master and, more importantly, triggered by a new release being published.

I think it should only restrict the behavior of builds for PRs.

[jameslamb]

Excellent, thank you so much!

I understand, we have a similar system at NVIDIA:

• add feature_names_in_ rapidsai/cuml#5678 (comment)
• add feature_names_in_ rapidsai/cuml#5678 (comment)

If this will not prevent us from triggering runs on merges and releases, and now that you've changed those settings to hopefully not require these comments for PRs from contributors, then I think this should not be too much additional friction.

Thanks very much for helping to unblock CI!

[jameslamb]

🎉 passing CI! Thanks for the help @shiyu1994 !

[jameslamb]

I just merged master into #6859 (a PR I opened, with just commits from me), and saw it automatically trigger an Azure DevOps job!

Also saw this build triggered on the merge of this PR to master: https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=17706&view=results

So I think the settings change from #6872 (comment) is working as expected.

[StrikerRUS]

@jameslamb Just discovered that I have full access to the repository settings. And seems you too.

[jameslamb]

@StrikerRUS oh great! Ok that is helpful. It looks like we still don't have access to configure apps like the lock-bot, but most other things.