patchGNU Libtool - Patches: patch #9999, Fix --preserve-dup-deps flag not...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9999: Fix --preserve-dup-deps flag not to strip some duplicates

Submitter:  Julien ÉLIE <iulius>
Submitted:  Sun 22 Nov 2020 11:03:40 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed

Jump to the original submission

Thu 27 Jun 2024 02:35:21 PM UTC, comment #6: 

This has been updated in development, and thank you again for your patch!

https://git.savannah.gnu.org/cgit/libtool.git/commit/?h=development&id=e517cfe7e1f11d5b5d99f63554521e239d5219e8

Ileana Dumitrescu <ildumi>
Group administrator
Wed 26 Jun 2024 05:37:15 PM UTC, comment #5: 

Yes I agree with you.  On second thoughts, the "FIXME" comment should remain.

Julien ÉLIE <iulius>
Tue 25 Jun 2024 01:37:32 PM UTC, comment #4: 

I will remove the INN comment, but if I understand the "FIXME" correctly, it should be kept as a reminder to rethink the logic of the code in this section. It does not seem to be specific to the 'preserve-dup-deps' flag. Please tell me if I am wrong though.

Ileana Dumitrescu <ildumi>
Group administrator
Sat 22 Jun 2024 09:23:35 AM UTC, comment #3: 

Many thanks!

I've had a look at the commit in the development branch.
I suggest to remove the comment "# Modification for INN in the loop (fix --preserve-dup-deps)."

I had put it in our locally modified version of ltmain.sh in INN so as to remember the code differed here from upstream.  It is no longer necessary, now it has been merged upstream.  I would then just remove the comment line.

Also, the "FIXME" could be removed.  The problem is fixed!


          # FIXME: Pedantically, this is the right thing to do, so
          #        that some nasty dependency loop isn't accidentally
          #        broken: new_libs="$deplib $new_libs"
          for deplib in $tmp_libs; do
            # Modification for INN in the loop (fix --preserve-dup-deps).
            if $opt_preserve_dup_deps; then
              new_libs="$deplib $new_libs"


would become:


          # Pedantically, this is the right thing to do, so
          # that some nasty dependency loop isn't accidentally
          # broken: new_libs="$deplib $new_libs"
          for deplib in $tmp_libs; do
            if $opt_preserve_dup_deps; then
              new_libs="$deplib $new_libs"


Have a nice day.

Julien ÉLIE <iulius>
Thu 20 Jun 2024 01:38:15 PM UTC, comment #2: 

Thank you for your patch! It has been merged in, and a test has been fixed along with it :)

Ileana Dumitrescu <ildumi>
Group administrator
Sun 21 Apr 2024 08:26:01 PM UTC, comment #1: 

FWIW, the patch was locally applied to INN in 2020, and there hasn't been any build problem since then.  Building INN with libtool otherwise failed with unresolved circular dependencies, even with the use of --preserve-dup-deps.

https://github.com/InterNetNews/inn/commit/e715e957fe7db5acc9ec1beaa9abb2ba120c287b#diff-6858ef1035c11e53d6a00ef454d86e2eab93aae2fc7f83f201069946e916c643


I believe it would be useful to fix it upstream too.  There even was a comment in ltmain.sh noting that the current code is broken and should be fixed, along with the fix in a commented line:


# FIXME: Pedantically, this is the right thing to do, so
#        that some nasty dependency loop isn't accidentally
#        broken:
#new_libs="$deplib $new_libs"


The patch basically uses the right expected logic for --preserve-dup-deps.

Julien ÉLIE <iulius>
Sun 22 Nov 2020 11:03:40 AM UTC, original submission:  

Hi all,

We've recently encountered a build issue because of circular dependencies that were not solved, even with the use of --preserve-dup-deps in Libtool.  It appears that --preserve-dup-deps does not do what it is supposed to do because of a broken optimization (marked as FIXME in the libtool code).

Suggestion of patch:

--- ltmain.sh.orig 2020-11-21 08:16:10.480694237 +0100
+++ ltmain.sh 2020-11-21 08:43:00.848365987 +0100
@@ -8686,41 +8686,45 @@
    eval tmp_libs=\"\$$var\"
    new_libs=
    for deplib in $tmp_libs; do
-     # FIXME: Pedantically, this is the right thing to do, so
-     #        that some nasty dependency loop isn't accidentally
-     #        broken:
-     #new_libs="$deplib $new_libs"
-     # Pragmatically, this seems to cause very few problems in
-     # practice:
-     case $deplib in
-     -L*) new_libs="$deplib $new_libs" ;;
-     -R*) ;;
-     *)
-       # And here is the reason: when a library appears more
-       # than once as an explicit dependence of a library, or
-       # is implicitly linked in more than once by the
-       # compiler, it is considered special, and multiple
-       # occurrences thereof are not removed.  Compare this
-       # with having the same library being listed as a
-       # dependency of multiple other libraries: in this case,
-       # we know (pedantically, we assume) the library does not
-       # need to be listed more than once, so we keep only the
-       # last copy.  This is not always right, but it is rare
-       # enough that we require users that really mean to play
-       # such unportable linking tricks to link the library
-       # using -Wl,-lname, so that libtool does not consider it
-       # for duplicate removal.
-       case " $specialdeplibs " in
-       " $deplib ") new_libs="$deplib $new_libs" ;;
+            if $opt_preserve_dup_deps; then
+       # Pedantically, this is the right thing to do, so
+       # that some nasty dependency loop isn't accidentally
+       # broken.
+       new_libs="$deplib $new_libs"
+            else
+       # Pragmatically, this seems to cause very few problems in
+       # practice:
+       case $deplib in
+       -L*) new_libs="$deplib $new_libs" ;;
+       -R*) ;;
        *)
- case " $new_libs " in
- " $deplib ") ;;
- *) new_libs="$deplib $new_libs" ;;
- esac
- ;;
-       esac
-       ;;
-     esac
+         # And here is the reason: when a library appears more
+         # than once as an explicit dependence of a library, or
+         # is implicitly linked in more than once by the
+         # compiler, it is considered special, and multiple
+         # occurrences thereof are not removed.  Compare this
+         # with having the same library being listed as a
+         # dependency of multiple other libraries: in this case,
+         # we know (pedantically, we assume) the library does not
+         # need to be listed more than once, so we keep only the
+         # last copy.  This is not always right, but it is rare
+         # enough that we require users that really mean to play
+         # such unportable linking tricks to link the library
+         # using -Wl,-lname, so that libtool does not consider it
+         # for duplicate removal.  And if not possible for portability
+                # reasons, then --preserve-dup-deps should be used.
+         case " $specialdeplibs " in
+         " $deplib ") new_libs="$deplib $new_libs" ;;
+         *)
+   case " $new_libs " in
+   " $deplib ") ;;
+   *) new_libs="$deplib $new_libs" ;;
+   esac
+   ;;
+         esac
+         ;;
+             esac
+            fi
    done
    tmp_libs=
    for deplib in $new_libs; do



Works OK with that patch.
Thanks for your work on Libtool,

--
Julien ÉLIE

Julien ÉLIE <iulius>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ildumi (Posted a comment)
  • -email is unavailable- added by iulius (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-06-27 ildumi StatusNone Done
    2024-06-20 ildumi Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code