Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[patch] libtool copies objects for unification which slows down build #4173

Closed
mc-butler opened this issue Jan 12, 2021 · 9 comments
Closed
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4173
Reporter psprint (sg44567@….com)

Hi,
I've had enough straying at the make message:

copying selected object files to avoid basename conflicts...

and I've decided to fix it – i.e.: to make the names of objects unique – patch is attached. It renames things such as lib/util.c and src/util.c so that their basename is unique (lib/sub-util.c in this case, for subroutine library…).

However, this is not enough to make the message (and the pause) go away. I've added following snippet into mc/libtool at line 11122 to get the names of the objects that still conflict:

            for obj in $oldobjs
	    do
	      func_basename "$obj"
	      $ECHO "$func_basename_result"
	    done | sort | sort >/tmp/libtool-info.txt 2>&1

and it turned out that there are plenty of such files. It looks like if some internal static library has been linked twice. I cannot go further than this point. Could anyone help?

I've tried removing libmc.la from:

– src/diffviewer/Makefile.am,
– src/filemanager/Makefile.am,
– src/viewer/Makefile.am,

the only places where it is (excessively, apparently) added to the linked libs but the effect is the same and the libtool-info.txt list doesn't change. Funny, isn't it?

Also, it looks like the main libmc.la reference is also not needed somehow, because changing:

mc_LDADD = \
	libinternal.la \
	$(top_builddir)/lib/libmc.la

to:

mc_LDADD = \
	libinternal.la

doesn't result in any build or runtime error. Funny again, isn't it?

From where do the duplicates come from? Has anyone some idea?

Note

Original attachments:

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Jan 12, 2021 at 19:12 UTC

a patch that makes all conflicting source files unique

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Jan 12, 2021 at 19:13 UTC

libtool trace on the list of objects that ·should· be unique

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 15, 2021 at 15:35 UTC (comment 1)

I don't think non-unique file names is a problem.
Apparently close as wontfix.

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Jan 20, 2021 at 18:03 UTC (comment 2)

Little regret, though… The filenames should be unique, because otherwise libtool is required to ·copy· the conflicting files under unique temporary names. Not sure, if this is only copying the names that conflict or other too, however an error in the Makefile.am files is making libmc to be linked three times so a large number of files are required to be copied anyway, and this adds 5-10 seconds to the build process (the copying object files to avoid basename conflicts… stage), which is a serious inconvenience (especially when the build is a simple non-header file change, inducing only a single file compilation and then an immediate link…)… The above patch resolves the real (not from linking multiple times) base name conflicts, so IMO it could/should be merged. Then the error in Makefile.am files could be tracked down.

PS. I've created a 100 points bounty for this problem on SO, maybe this will help solving the issue?

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Jan 23, 2021 at 14:24 UTC (comment 3)

I've revised the problem and was able to successfully get rid of the "Copying object … avoid conflicts" stage of the linking! Time of the build dropped from 2:50 seconds to 2:34 seconds meaning that the time of build after basic single .c file change will be shorter by 16 seconds :) Please merge, the patch is safe as it doesn't do any logic changes, it only renames the files and – crucial change: – removes libmc.a from libdiffviewer.a and libmcviewer.a, because they are an unneeded, excessive linkage that were causing the objects of libmc.a to be linked 3 times… Without it and with the few renames, the linking is free of basename conflicts and can go without the copy stage.

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Jan 23, 2021 at 14:25 UTC

@mc-butler
Copy link
Author

Changed by psprint (sg44567@….com) on Feb 8, 2021 at 22:24 UTC (comment 4)

Ping on this important patch, which shortens build time. Will it be merged? It's completely safe patch – no logic is being introduced by it.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 9, 2021 at 7:39 UTC (comment 5)

Libraries linkage is fixed in the 4179_cleanup branch (current changeset:[[95f92ece391000e88d2c65019a889dfa2c2f8136]).
This branch will be merged to master after a while.
Files will not be renamed.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Mar 14, 2021 at 17:07 UTC (comment 6)

  • Resolution set to fixed
  • Milestone changed from Future Releases to 4.8.27
  • Status changed from new to closed

[f29533f]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress
Development

No branches or pull requests

1 participant