Actually I got a report from @chjinche this morning about the issue. And the first correct path is used in the LightGBM-Transform. So, can we just keep this since it seems that LightGBM-Transform heavily relies on the first path.

https://github.com/microsoft/LightGBM-Transform

If we want to prioritize the safety for the package, @chjinche may also help to change the paths in LightGBM-Transform. Which approach do you think is preferred? @jameslamb @chjinche

Actually I got a report from @chjinche this morning about the issue

Thanks for that. In the future @chjinche , I'd appreciate if you please post here on the project's public issue tracker instead of sending bug reports in private messages.

can we just keep this since it seems that LightGBM-Transform heavily relies on the first path

I'm surprised to learn that project relies "heavily" on something that appears to have been broken for several months.

Seems that they're just now picking up development after more than a year of inactivity. https://github.com/microsoft/lightgbm-transform/commits/main

I think LightGBM-Transform should be changed to adapt to what LightGBM does, and not the other way around. And that we should take this opportunity to slightly simplify library-loading for lightgbm.

hi @jameslamb to clarify, we are not asking for some private message, but are debugging community PR CI failure microsoft/lightgbm-transform#26 issue and found this.

Looks it is a fault as you mentioned, and not intentional, could you please allow this fix in this time, so unblock people, we may consider improving our releasing process in the future to be more tolerant for the candidate path changes.

are debugging community PR CI failure microsoft/lightgbm-transform#26 issue and found this

Thanks for that! Linking to that context in the PR description would have prevented me from drawing an incorrect conclusion.

could you please allow this fix in this time

I really don't think we should.

It seems that all you'd need to do is to fix these lines:

https://github.com/microsoft/lightgbm-transform/blob/1d7d34265954dc872c37512699e4c25aa27f7906/.ci/setup.sh#L11

https://github.com/microsoft/lightgbm-transform/blob/1d7d34265954dc872c37512699e4c25aa27f7906/scripts/publish_python_package.sh#L10

to copy lib_lightgbm.so to one of the other supported paths.

I don't support missing this opportunity to simplify lightgbm just for LightGBM-Transform to avoid having to make such a small change to a CI script. It is LightGBM-Transform that's using a custom build process (compiling lib_lightgbm.so and then manually moving it around with cp), and that should conform to how LightGBM works.

Thanks @jameslamb, but looks more than the line work. This is because transform lib works as extensive so(lib_custom_parser.so, lib_transform.so) together with lib_transform.so, and also need some basic.py modification.

Not sure if just simple line modification works, but I'll try. Anyway, cur_path (/LightGBM/python-package/lightgbm/libpath.py/lib_lightgbm.so) is invalid, right? Why not restore to previous valid path?

Why not restore to previous valid path?

Because as far as I can tell, the only thing relying on lib_lightgbm.so existing at that path is LightGBM-Transform.

Please see all the discussion in #5061 and all the pull requests linked from it. In response to changes in Python packaging standards upstream (e.g. in pip, build, and wheel), we did significant work to remove non-standard features of lightgbm's packaging setup. One of those non-standard features was the fact that it permitted lib_lightgbm.so to be found in so many different places.

That previous path (lightgbm/lib_lightgbm.so) was left behind only in an effort to minimize as much as possible the number of changes I was proposing. I didn't carefully scrutinize each path (which is how the mistake this PR exposes was allowed to persist).

But now that I am looking very carefully at each path as a result of this PR, I'd prefer to remove a path that, as far as I can tell, only LightGBM-Transform relies on.

Having fewer possible places that lib_lightgbm.so can be found has the following benefits:

• speeds up library loading
• reduces the risk of accidentally loading the wrong library (e.g. one left over by some other build of an older LightGBM version)

Ultimately @shiyu1994 has the power to override me in this project, so if he says to drop my objection then I'll drop it.

But I feel strongly that making lightgbm safer is preferable to keeping around a rarely-used path only for the benefit of LightGBM-Transform.

@jameslamb if I understand correctly, the first candidate path should be removed, right? Can you commit the other three will not change in the future? Please kindly share your future plan.

1. LightGBM/python-package/lightgbm/libpath.py/lib_lightgbm.so
2. LightGBM/python-package/lib_lightgbm.so
3. LightGBM/python-package/lightgbm/bin/lib_lightgbm.so
4. LightGBM/python-package/lightgbm/lib/lib_lightgbm.so

the first candidate path should be removed, right

correct.

can you commit the other three will not change in the future

I have no plan to remove those, correct.

Based on microsoft/lightgbm-transform#26 being merged and the changes that were passed there, I guess you were able to resolve this in lightgbm-transform @chjinche . Thank you!

@shiyu1994 could you please update this PR to just completely remove that first path? Let's take this opportunity to simplify lightgbm.

Thanks. I've removed the first library path.

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.