From 79373c9bf92ad0307dbc02415cd389cded2bc1f8 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 12:46:48 +0900 Subject: [PATCH 1/9] Add mergeback for -D and -W cflags to match pkgconf behavior When multiple packages have the same -D or -W flags, pkgconf removes previous occurrences and keeps only the last one (mergeback). This prevents flag duplication like "-DNOMINMAX" appearing many times. ```console$ pkgconf --cflags re2 -pthread -DNOMINMAX $ ruby -r pkg-config -e 'puts PKGConfig.cflags("re2")' -pthread -DNOMINMAX -DNOMINMAX -DNOMINMAX ... ``` Flags excluded from mergeback (kept as duplicates): - -Wa, (assembler options) - -Wl, (linker options) - -Wp, (preprocessor options) This PR only supports mergeback for -D and -W flags. Other flags that pkgconf also mergebacks are not yet supported. --- lib/pkg-config.rb | 22 +++++++ test/test-pkg-config.rb | 123 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index 197af8b..7c9d02f 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -545,6 +545,7 @@ def collect_cflags flag.gsub(/\A-I/, "/I") end end + other_flags = mergeback_flags(other_flags) [path_flags, other_flags] end @@ -579,6 +580,27 @@ def normalize_cflags(cflags) normalized_cflags end + def mergeback_flags(flags) + mergebacked_flags = [] + flags.each do |flag| + if mergeable_flag?(flag) + mergebacked_flags.delete(flag) + end + mergebacked_flags << flag + end + mergebacked_flags + end + + def mergeable_flag?(flag) + return false unless flag.start_with?("-") + return true if flag.start_with?("-D") + if flag.start_with?("-W") + return false if flag.start_with?("-Wa,", "-Wl,", "-Wp,") + return true + end + false + end + def collect_libs target_packages = [*required_packages, self] libs_set = [] diff --git a/test/test-pkg-config.rb b/test/test-pkg-config.rb index eace04b..75735bf 100644 --- a/test/test-pkg-config.rb +++ b/test/test-pkg-config.rb @@ -314,4 +314,127 @@ def test_equals_to parse_requires("fribidi = 1.0")) end end + + sub_test_case("cflags mergeback") do + def create_pc_file(name, content) + pc_dir = File.join(@tmpdir, "pkgconfig") + FileUtils.mkdir_p(pc_dir) + pc_path = File.join(pc_dir, "#{name}.pc") + File.write(pc_path, content) + pc_dir + end + + def setup + PackageConfig.clear_configure_args_cache + @tmpdir = Dir.mktmpdir + end + + def teardown + FileUtils.rm_rf(@tmpdir) + end + + def test_d_flag_mergeback + pc_dir = create_pc_file("main-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: main-package +Description: Main package for testing +Version: 1.0.0 +Requires: dep-package +Cflags: -I${includedir}/main -DFOO + PC + create_pc_file("dep-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: dep-package +Description: Dependency package +Version: 1.0.0 +Cflags: -I${includedir}/dep -DFOO + PC + package = PackageConfig.new("main-package", paths: [pc_dir]) + assert_equal("-I/usr/local/include/main -I/usr/local/include/dep -DFOO", + package.cflags) + end + + def test_w_flag_mergeback + pc_dir = create_pc_file("main-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: main-package +Description: Main package for testing +Version: 1.0.0 +Requires: dep-package +Cflags: -I${includedir}/main -Wno-unknown-warning-option + PC + create_pc_file("dep-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: dep-package +Description: Dependency package +Version: 1.0.0 +Cflags: -I${includedir}/dep -Wno-unknown-warning-option + PC + package = PackageConfig.new("main-package", paths: [pc_dir]) + assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + + "-Wno-unknown-warning-option", + package.cflags) + end + + def test_wa_wl_wp_flags_not_mergebacked + pc_dir = create_pc_file("main-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: main-package +Description: Main package for testing +Version: 1.0.0 +Requires: dep-package +Cflags: -I${includedir}/main -Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO + PC + create_pc_file("dep-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: dep-package +Description: Dependency package +Version: 1.0.0 +Cflags: -I${includedir}/dep -Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO + PC + package = PackageConfig.new("main-package", paths: [pc_dir]) + assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + + "-Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO " + + "-Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO", + package.cflags) + end + + def test_mixed_flags_mergeback + pc_dir = create_pc_file("main-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: main-package +Description: Main package for testing +Version: 1.0.0 +Requires: dep-package +Cflags: -I${includedir}/main -DFOO -Wall -Wl,--as-needed + PC + create_pc_file("dep-package", <<-PC) +prefix=/usr/local +includedir=${prefix}/include + +Name: dep-package +Description: Dependency package +Version: 1.0.0 +Cflags: -I${includedir}/dep -DFOO -Wall -Wl,--as-needed + PC + package = PackageConfig.new("main-package", paths: [pc_dir]) + assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + + "-Wl,--as-needed -DFOO -Wall -Wl,--as-needed", + package.cflags) + end + end end From 19cd8b85e1e318021e07aaf0b42ee3a956e98f39 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 15:50:53 +0900 Subject: [PATCH 2/9] Added the comment about the method's specification --- lib/pkg-config.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index 7c9d02f..e49948a 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -580,6 +580,9 @@ def normalize_cflags(cflags) normalized_cflags end + # Implementing behavior compatible with pkgconf's pkgconf_fragment_copy(). + # This is not a complete reproduction yet, but the goal is to stay compatible. + # https://github.com/pkgconf/pkgconf/blob/pkgconf-2.5.1/libpkgconf/fragment.c#L381-L416 def mergeback_flags(flags) mergebacked_flags = [] flags.each do |flag| From de47ab707c9d3167175f2c919c1779c7c58be6f9 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 15:57:11 +0900 Subject: [PATCH 3/9] Add the note about the speed of the function --- lib/pkg-config.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index e49948a..81065ef 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -583,6 +583,9 @@ def normalize_cflags(cflags) # Implementing behavior compatible with pkgconf's pkgconf_fragment_copy(). # This is not a complete reproduction yet, but the goal is to stay compatible. # https://github.com/pkgconf/pkgconf/blob/pkgconf-2.5.1/libpkgconf/fragment.c#L381-L416 + # + # NOTE: This may be slow because this checks mergebacked_flags N times (where + # N is the number of mergeable flags). def mergeback_flags(flags) mergebacked_flags = [] flags.each do |flag| From c6b9da8a70382f564cf8c97581ee35ecc78057e7 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:04:39 +0900 Subject: [PATCH 4/9] Manipulate the only cflags here --- lib/pkg-config.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index 81065ef..01bc008 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -534,7 +534,7 @@ def collect_cflags cflags_set << package.declaration("Cflags") end all_cflags = normalize_cflags(Shellwords.split(cflags_set.join(" "))) - path_flags, other_flags = all_cflags.partition {|flag| /\A-I/ =~ flag} + path_flags, cflags = all_cflags.partition {|flag| /\A-I/ =~ flag} path_flags = path_flags.collect {|flag| normalize_path_flag(flag, "-I")} path_flags = path_flags.reject do |flag| flag == "-I/usr/include" @@ -545,8 +545,8 @@ def collect_cflags flag.gsub(/\A-I/, "/I") end end - other_flags = mergeback_flags(other_flags) - [path_flags, other_flags] + cflags = merge_back_cflags(cflags) + [path_flags, cflags] end def normalize_path_flag(path_flag, flag_option) @@ -584,17 +584,17 @@ def normalize_cflags(cflags) # This is not a complete reproduction yet, but the goal is to stay compatible. # https://github.com/pkgconf/pkgconf/blob/pkgconf-2.5.1/libpkgconf/fragment.c#L381-L416 # - # NOTE: This may be slow because this checks mergebacked_flags N times (where + # NOTE: This may be slow because this checks merge_back_cflags N times (where # N is the number of mergeable flags). - def mergeback_flags(flags) - mergebacked_flags = [] - flags.each do |flag| - if mergeable_flag?(flag) - mergebacked_flags.delete(flag) + def merge_back_cflags(cflags) + mergebacked_cflags = [] + cflags.each do |cflag| + if mergeable_flag?(cflag) + mergebacked_cflags.delete(cflag) end - mergebacked_flags << flag + mergebacked_cflags << cflag end - mergebacked_flags + mergebacked_cflags end def mergeable_flag?(flag) From 062f4be0d6a28608f13b0214b2286d8fb4604dfb Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:09:45 +0900 Subject: [PATCH 5/9] Simplify the test cases --- test/test-pkg-config.rb | 131 ++++++---------------------------------- 1 file changed, 19 insertions(+), 112 deletions(-) diff --git a/test/test-pkg-config.rb b/test/test-pkg-config.rb index 75735bf..53ed04a 100644 --- a/test/test-pkg-config.rb +++ b/test/test-pkg-config.rb @@ -315,126 +315,33 @@ def test_equals_to end end - sub_test_case("cflags mergeback") do - def create_pc_file(name, content) - pc_dir = File.join(@tmpdir, "pkgconfig") - FileUtils.mkdir_p(pc_dir) - pc_path = File.join(pc_dir, "#{name}.pc") - File.write(pc_path, content) - pc_dir + sub_test_case("#merge_back_cflags") do + def merge_back_cflags(cflags) + @glib.__send__(:merge_back_cflags, cflags) end - def setup - PackageConfig.clear_configure_args_cache - @tmpdir = Dir.mktmpdir + def test_d_flag + assert_equal(["-DFOO"], + merge_back_cflags(["-DFOO", "-DFOO"])) end - def teardown - FileUtils.rm_rf(@tmpdir) + def test_w_flag + assert_equal(["-Wno-unknown-warning-option"], + merge_back_cflags(["-Wno-unknown-warning-option", + "-Wno-unknown-warning-option"])) end - def test_d_flag_mergeback - pc_dir = create_pc_file("main-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: main-package -Description: Main package for testing -Version: 1.0.0 -Requires: dep-package -Cflags: -I${includedir}/main -DFOO - PC - create_pc_file("dep-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: dep-package -Description: Dependency package -Version: 1.0.0 -Cflags: -I${includedir}/dep -DFOO - PC - package = PackageConfig.new("main-package", paths: [pc_dir]) - assert_equal("-I/usr/local/include/main -I/usr/local/include/dep -DFOO", - package.cflags) - end - - def test_w_flag_mergeback - pc_dir = create_pc_file("main-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: main-package -Description: Main package for testing -Version: 1.0.0 -Requires: dep-package -Cflags: -I${includedir}/main -Wno-unknown-warning-option - PC - create_pc_file("dep-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: dep-package -Description: Dependency package -Version: 1.0.0 -Cflags: -I${includedir}/dep -Wno-unknown-warning-option - PC - package = PackageConfig.new("main-package", paths: [pc_dir]) - assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + - "-Wno-unknown-warning-option", - package.cflags) + def test_wa_wl_wp_flags_not_merged_back + assert_equal(["-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO", + "-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO"], + merge_back_cflags(["-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO", + "-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO"])) end - def test_wa_wl_wp_flags_not_mergebacked - pc_dir = create_pc_file("main-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: main-package -Description: Main package for testing -Version: 1.0.0 -Requires: dep-package -Cflags: -I${includedir}/main -Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO - PC - create_pc_file("dep-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: dep-package -Description: Dependency package -Version: 1.0.0 -Cflags: -I${includedir}/dep -Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO - PC - package = PackageConfig.new("main-package", paths: [pc_dir]) - assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + - "-Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO " + - "-Wa,--noexecstack -Wl,--as-needed -Wp,-DFOO", - package.cflags) - end - - def test_mixed_flags_mergeback - pc_dir = create_pc_file("main-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: main-package -Description: Main package for testing -Version: 1.0.0 -Requires: dep-package -Cflags: -I${includedir}/main -DFOO -Wall -Wl,--as-needed - PC - create_pc_file("dep-package", <<-PC) -prefix=/usr/local -includedir=${prefix}/include - -Name: dep-package -Description: Dependency package -Version: 1.0.0 -Cflags: -I${includedir}/dep -DFOO -Wall -Wl,--as-needed - PC - package = PackageConfig.new("main-package", paths: [pc_dir]) - assert_equal("-I/usr/local/include/main -I/usr/local/include/dep " + - "-Wl,--as-needed -DFOO -Wall -Wl,--as-needed", - package.cflags) + def test_mixed_flags + assert_equal(["-Wl,--as-needed", "-DFOO", "-Wall", "-Wl,--as-needed"], + merge_back_cflags(["-DFOO", "-Wall", "-Wl,--as-needed", + "-DFOO", "-Wall", "-Wl,--as-needed"])) end end end From 5230529c9b20edf441220d4159de15336fa9766f Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:21:21 +0900 Subject: [PATCH 6/9] Renamed the variable name to other_flags It's because this method collect cflags, so all of them are cflags too. --- lib/pkg-config.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index 01bc008..d0849e4 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -534,7 +534,7 @@ def collect_cflags cflags_set << package.declaration("Cflags") end all_cflags = normalize_cflags(Shellwords.split(cflags_set.join(" "))) - path_flags, cflags = all_cflags.partition {|flag| /\A-I/ =~ flag} + path_flags, other_flags = all_cflags.partition {|flag| /\A-I/ =~ flag} path_flags = path_flags.collect {|flag| normalize_path_flag(flag, "-I")} path_flags = path_flags.reject do |flag| flag == "-I/usr/include" @@ -545,8 +545,8 @@ def collect_cflags flag.gsub(/\A-I/, "/I") end end - cflags = merge_back_cflags(cflags) - [path_flags, cflags] + other_flags = merge_back_cflags(other_flags) + [path_flags, other_flags] end def normalize_path_flag(path_flag, flag_option) From b9a7cdcef82a5286d4be1449878dac60077630c9 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:31:20 +0900 Subject: [PATCH 7/9] Moved the comment to the appropriate place --- lib/pkg-config.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index d0849e4..8ebcf5c 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -583,13 +583,12 @@ def normalize_cflags(cflags) # Implementing behavior compatible with pkgconf's pkgconf_fragment_copy(). # This is not a complete reproduction yet, but the goal is to stay compatible. # https://github.com/pkgconf/pkgconf/blob/pkgconf-2.5.1/libpkgconf/fragment.c#L381-L416 - # - # NOTE: This may be slow because this checks merge_back_cflags N times (where - # N is the number of mergeable flags). def merge_back_cflags(cflags) mergebacked_cflags = [] cflags.each do |cflag| if mergeable_flag?(cflag) + # NOTE: This may be slow because this checks merge_back_cflags N times + # (where N is the number of mergeable flags). mergebacked_cflags.delete(cflag) end mergebacked_cflags << cflag From 6e7d245bdf9cc2aa26ed59495abd1bb416c9e923 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:32:24 +0900 Subject: [PATCH 8/9] Fixed the typo --- lib/pkg-config.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/pkg-config.rb b/lib/pkg-config.rb index 8ebcf5c..d46994e 100644 --- a/lib/pkg-config.rb +++ b/lib/pkg-config.rb @@ -584,16 +584,16 @@ def normalize_cflags(cflags) # This is not a complete reproduction yet, but the goal is to stay compatible. # https://github.com/pkgconf/pkgconf/blob/pkgconf-2.5.1/libpkgconf/fragment.c#L381-L416 def merge_back_cflags(cflags) - mergebacked_cflags = [] + merge_backed_cflags = [] cflags.each do |cflag| if mergeable_flag?(cflag) # NOTE: This may be slow because this checks merge_back_cflags N times # (where N is the number of mergeable flags). - mergebacked_cflags.delete(cflag) + merge_backed_cflags.delete(cflag) end - mergebacked_cflags << cflag + merge_backed_cflags << cflag end - mergebacked_cflags + merge_backed_cflags end def mergeable_flag?(flag) From c21530f1454f285598842db935eeec0c35184011 Mon Sep 17 00:00:00 2001 From: otegami Date: Mon, 1 Dec 2025 16:33:33 +0900 Subject: [PATCH 9/9] Remove the redundatant words from tests --- test/test-pkg-config.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test-pkg-config.rb b/test/test-pkg-config.rb index 53ed04a..67893b3 100644 --- a/test/test-pkg-config.rb +++ b/test/test-pkg-config.rb @@ -320,25 +320,25 @@ def merge_back_cflags(cflags) @glib.__send__(:merge_back_cflags, cflags) end - def test_d_flag + def test_d assert_equal(["-DFOO"], merge_back_cflags(["-DFOO", "-DFOO"])) end - def test_w_flag + def test_w assert_equal(["-Wno-unknown-warning-option"], merge_back_cflags(["-Wno-unknown-warning-option", "-Wno-unknown-warning-option"])) end - def test_wa_wl_wp_flags_not_merged_back + def test_wa_wl_wp assert_equal(["-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO", "-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO"], merge_back_cflags(["-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO", "-Wa,--noexecstack", "-Wl,--as-needed", "-Wp,-DFOO"])) end - def test_mixed_flags + def test_mixed assert_equal(["-Wl,--as-needed", "-DFOO", "-Wall", "-Wl,--as-needed"], merge_back_cflags(["-DFOO", "-Wall", "-Wl,--as-needed", "-DFOO", "-Wall", "-Wl,--as-needed"]))