fix!: resolve issue with relative paths during linking

[Dw9]

nodejs/node-gyp#3128

[legendecas]

LGTM.

Searching through usages on library_dirs: https://github.com/search?type=code&q=library_dirs+path%253A*.gyp. It seems to me there are relative paths in library_dirs in the wild that will be break by this change. I think this PR should be released as a breaking change.

[legendecas]

Would you mind updating the list in the document https://github.com/nodejs/gyp-next/blob/main/docs/InputFormatReference.md#pathname-relativization as well?

library_dirs is suffixed with _dirs, should be treated as pathnames and be included in pathname relativization. As it is built-in config key, I believe it is better to explicitly list it here.

Thank you!

[legendecas]

• cmake:

  gyp-next/pylib/gyp/generator/cmake.py

  Line 817 in 46fc91f

  output.write(NormjoinPath(path_from_cmakelists_to_gyp, library_dir))

• ninja:

  gyp-next/pylib/gyp/generator/ninja.py

  Line 1491 in 46fc91f

  QuoteShellArgument("-L" + self.GypPathToNinja(library_dir), self.flavor)

• msvc:

  gyp-next/pylib/gyp/generator/msvs.py

  Line 3354 in 46fc91f

  library_dirs = _GetLibraryDirs(configuration)

Seems like we don't have tests on this, but it will be great if you can update these generators as well to keep them in align.

[Dw9]

hi～@legendecas； I'm not very familiar with the project and don't know how to test with cmake/nanja/msvc. Could you please advise on how to make modifications?

[Dw9]

hi～ any updates here

[legendecas]

Generators can be tested with option --format, like gyp -f ninja.

Setting up the integration test in #285.

[legendecas]

@Dw9 would you mind rebasing this on top of the main branch and update the integration tests? Thank you! From what's been captured in the integration test, updating make is sufficient, as both ninja and cmake handle the library_dirs correctly.

gyp-next/test/fixtures/expected-linux/ninja/test.ninja

Line 11 in 356f09c

libs = -L../../mylib

gyp-next/test/fixtures/expected-linux/cmake/CMakeLists.txt

Line 19 in 356f09c

link_directories( ../../mylib

[Dw9]

Tested locally. All good.Thank you for your work！ @legendecas

[Dw9]

CI build failed in Python tests 😅

[cclauss]

@legendecas Perhaps assert_file() needs to do a more line-by-line diff to make the output is more readable.

gyp-next/test/integration_test.py

Line 26 in 356f09c

def assert_file(self, actual, expected) -> None:

[legendecas]

Would you mind updating the integration test? like

gyp-next/test/fixtures/expected-darwin/make/test.target.mk

Line 64 in 356f09c

-Lmylib

gyp-next/test/fixtures/expected-linux/make/test.target.mk

Line 47 in 356f09c

-Lmylib

[Dw9]

Thank you for your guidance and patience. @legendecas

[Dw9]

gyp-next git:(main) pytest -vv
======================================================================== test session starts =========================================================================
platform darwin -- Python 3.12.4, pytest-7.4.4, pluggy-1.0.0 -- /opt/anaconda3/bin/python
cachedir: .pytest_cache
rootdir: /Users/weishao/WorkSpace/node-gyp/gyp-next
plugins: anyio-4.8.0, hydra-core-1.3.2
collected 36 items

pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testConvertToMSBuildSettings_actual <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                     [  2%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testConvertToMSBuildSettings_empty <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                      [  5%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testConvertToMSBuildSettings_full_synthetic <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED             [  8%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testConvertToMSBuildSettings_minimal <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                    [ 11%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testConvertToMSBuildSettings_warnings <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                   [ 13%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testValidateMSBuildSettings_settings <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                    [ 16%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testValidateMSVSSettings_settings <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                       [ 19%]
pylib/gyp/MSVSSettings_test.py::TestSequenceFunctions::testValidateMSVSSettings_tool_names <- ../gyp/pylib/gyp/MSVSSettings_test.py PASSED                     [ 22%]
pylib/gyp/common_test.py::TestTopologicallySorted::test_Cycle <- ../gyp/pylib/gyp/common_test.py PASSED                                                        [ 25%]
pylib/gyp/common_test.py::TestTopologicallySorted::test_Valid <- ../gyp/pylib/gyp/common_test.py PASSED                                                        [ 27%]
pylib/gyp/common_test.py::TestGetFlavor::test_GetCrossCompilerPredefines <- ../gyp/pylib/gyp/common_test.py PASSED                                             [ 30%]
pylib/gyp/common_test.py::TestGetFlavor::test_param <- ../gyp/pylib/gyp/common_test.py PASSED                                                                  [ 33%]
pylib/gyp/common_test.py::TestGetFlavor::test_platform_default <- ../gyp/pylib/gyp/common_test.py PASSED                                                       [ 36%]
pylib/gyp/easy_xml_test.py::TestSequenceFunctions::test_EasyXml_complex <- ../gyp/pylib/gyp/easy_xml_test.py PASSED                                            [ 38%]
pylib/gyp/easy_xml_test.py::TestSequenceFunctions::test_EasyXml_escaping <- ../gyp/pylib/gyp/easy_xml_test.py PASSED                                           [ 41%]
pylib/gyp/easy_xml_test.py::TestSequenceFunctions::test_EasyXml_pretty <- ../gyp/pylib/gyp/easy_xml_test.py PASSED                                             [ 44%]
pylib/gyp/easy_xml_test.py::TestSequenceFunctions::test_EasyXml_simple <- ../gyp/pylib/gyp/easy_xml_test.py PASSED                                             [ 47%]
pylib/gyp/easy_xml_test.py::TestSequenceFunctions::test_EasyXml_simple_with_attributes <- ../gyp/pylib/gyp/easy_xml_test.py PASSED                             [ 50%]
pylib/gyp/input_test.py::TestFindCycles::test_big_cycle <- ../gyp/pylib/gyp/input_test.py PASSED                                                               [ 52%]
pylib/gyp/input_test.py::TestFindCycles::test_cycle_self_reference <- ../gyp/pylib/gyp/input_test.py PASSED                                                    [ 55%]
pylib/gyp/input_test.py::TestFindCycles::test_cycle_two_nodes <- ../gyp/pylib/gyp/input_test.py PASSED                                                         [ 58%]
pylib/gyp/input_test.py::TestFindCycles::test_no_cycle_dag <- ../gyp/pylib/gyp/input_test.py PASSED                                                            [ 61%]
pylib/gyp/input_test.py::TestFindCycles::test_no_cycle_empty_graph <- ../gyp/pylib/gyp/input_test.py PASSED                                                    [ 63%]
pylib/gyp/input_test.py::TestFindCycles::test_no_cycle_line <- ../gyp/pylib/gyp/input_test.py PASSED                                                           [ 66%]
pylib/gyp/input_test.py::TestFindCycles::test_two_cycles <- ../gyp/pylib/gyp/input_test.py PASSED                                                              [ 69%]
pylib/gyp/xcode_emulation_test.py::TestXcodeSettings::test_GetCflags <- ../gyp/pylib/gyp/xcode_emulation_test.py PASSED                                        [ 72%]
pylib/gyp/xcode_emulation_test.py::TestXcodeSettings::test_GetLdflags <- ../gyp/pylib/gyp/xcode_emulation_test.py PASSED                                       [ 75%]
pylib/gyp/generator/msvs_test.py::TestSequenceFunctions::test_GetLibraries <- ../gyp/pylib/gyp/generator/msvs_test.py PASSED                                   [ 77%]
pylib/gyp/generator/ninja_test.py::TestPrefixesAndSuffixes::test_BinaryNamesLinux <- ../gyp/pylib/gyp/generator/ninja_test.py PASSED                           [ 80%]
pylib/gyp/generator/ninja_test.py::TestPrefixesAndSuffixes::test_BinaryNamesWindows <- ../gyp/pylib/gyp/generator/ninja_test.py PASSED                         [ 83%]
pylib/gyp/generator/ninja_test.py::TestPrefixesAndSuffixes::test_GenerateCompileDBWithNinja <- ../gyp/pylib/gyp/generator/ninja_test.py PASSED                 [ 86%]
pylib/gyp/generator/xcode_test.py::TestEscapeXcodeDefine::test_Escaping <- ../gyp/pylib/gyp/generator/xcode_test.py PASSED                                     [ 88%]
pylib/gyp/generator/xcode_test.py::TestEscapeXcodeDefine::test_InheritedRemainsUnescaped <- ../gyp/pylib/gyp/generator/xcode_test.py PASSED                    [ 91%]
test/integration_test.py::TestGyp::test_cmake PASSED                                                                                                           [ 94%]
test/integration_test.py::TestGyp::test_make PASSED                                                                                                            [ 97%]
test/integration_test.py::TestGyp::test_ninja PASSED                                                                                                           [100%]

========================================================================= 36 passed in 0.99s =========================================================================

[Dw9]

ci is green now.😄

[legendecas]

Thanks!