Page MenuHomeFreeBSD

The practice of creating symbolic links is somewhat fragile. Make hard copies instead always.
ClosedPublic

Authored by imp on Aug 5 2020, 9:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 4 2024, 11:08 AM
Unknown Object (File)
Jan 9 2024, 5:13 AM
Unknown Object (File)
Jan 9 2024, 5:13 AM
Unknown Object (File)
Jan 9 2024, 5:09 AM
Unknown Object (File)
Jan 9 2024, 4:51 AM
Unknown Object (File)
Dec 20 2023, 7:10 AM
Unknown Object (File)
Dec 16 2023, 11:11 PM
Unknown Object (File)
Nov 7 2023, 11:07 PM
Subscribers

Details

Summary

There's too many times that we can't run the new binaries with old
libraries. Making the links when things are known to be 'safe' is a
nice optimization, but a copy of all the binaries is only 30MB, so
saving the copies at the cost of increased support when new symbols
are added and used as part of the bootstrap seems to be unwise.

Test Plan

Note: this likely requires some comments to be adjusted.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32785
Build 30217: arc lint + arc unit

Event Timeline

imp requested review of this revision.Aug 5 2020, 9:09 PM
imp added reviewers: bdrewery, arichardson, kevans.
rpokala added inline comments.
Makefile.inc1
2289

Do we want to delete the old version before checking for the existence of the new? Or would it be better to check for the existence of the new, and then remove the old?

For that matter, since you're using cp -f, the deletion is already part of that: from cp(1):

-f    For each existing destination pathname, remove it and create a new
      file, without prompting for confirmation regardless of its
      permissions.  (The -f option overrides any previous -i or -n
      options.)
tools/build/Makefile
122

You introduced a @ in front of source_path; intentional, or accidental?

127

Same comment as above; cp -f means the rm is redundant.

136

And here.

140

And here too.

Makefile.inc1
2289

Yes. I do. And I have to do it first otherwise, the which will pick up the current copy here, defeating the purpose of the which...

tools/build/Makefile
122

Yes. Intentional. @ means to not print this command in make and the if had it before...

127

Here and below I got an error that the two files were identical w/o the -f flag.

tools/build/Makefile
127

Here and below I got an error that the two files were identical w/o the -f flag.

That's fine, include the -f, but in that case, the rm -f shouldn't be necessary.

Makefile.inc1
2289

Also, checking the source, the check for identical files occurs before the use fflag to do the copy anyway.

tools/build/Makefile
127

If I don't remove, then I get the error. The -f flag to rm is needed in case the file isn't here yet. the -f flag to cp may not truly be needed...

Copying instead of creating links *should* be fine. However, I vaguely seem to recall this previously caused issues for bootstrapping macOS since one tool wanted to find something relative to realpath argv[0]. Will check if that's still the case tomorrow morning UK time.

Copying instead of creating links *should* be fine. However, I vaguely seem to recall this previously caused issues for bootstrapping macOS since one tool wanted to find something relative to realpath argv[0]. Will check if that's still the case tomorrow morning UK time.

Sure thing. I included you because I had hoped to get this perspective. On Mac/Linux, only the incompatible tools need to be bootstrapped, and they will use $HOST libraries always, no?

In D25967#575682, @imp wrote:

Copying instead of creating links *should* be fine. However, I vaguely seem to recall this previously caused issues for bootstrapping macOS since one tool wanted to find something relative to realpath argv[0]. Will check if that's still the case tomorrow morning UK time.

Sure thing. I included you because I had hoped to get this perspective. On Mac/Linux, only the incompatible tools need to be bootstrapped, and they will use $HOST libraries always, no?

Yes, and they default to BOOTSTRAP_ALL_TOOLS.

In D25967#575682, @imp wrote:

Copying instead of creating links *should* be fine. However, I vaguely seem to recall this previously caused issues for bootstrapping macOS since one tool wanted to find something relative to realpath argv[0]. Will check if that's still the case tomorrow morning UK time.

Sure thing. I included you because I had hoped to get this perspective. On Mac/Linux, only the incompatible tools need to be bootstrapped, and they will use $HOST libraries always, no?

Yes, and they default to BOOTSTRAP_ALL_TOOLS.

So they never use the host native binaries once bootsrapped?

In D25967#575752, @imp wrote:
In D25967#575682, @imp wrote:

Copying instead of creating links *should* be fine. However, I vaguely seem to recall this previously caused issues for bootstrapping macOS since one tool wanted to find something relative to realpath argv[0]. Will check if that's still the case tomorrow morning UK time.

Sure thing. I included you because I had hoped to get this perspective. On Mac/Linux, only the incompatible tools need to be bootstrapped, and they will use $HOST libraries always, no?

Yes, and they default to BOOTSTRAP_ALL_TOOLS.

So they never use the host native binaries once bootsrapped?

Well the ALL in BOOTSTRAP_ALL_TOOLS is not quite true. Certain basic tools such as cp, mv, etc. are never bootstrapped.

Additionally, crossbuilding enforces -DNO_ROOT so the LD_LIBRARY_PATH issue during install doesn't show up.

I can confirm that cross-building from macOS still works with this patch. I have a slight preference for keeping symlinks in the ${.MAKE.OS} != "FreeBSD" / -DNO_ROOT case but keeping stuff consistent is more important.

This revision is now accepted and ready to land.Aug 6 2020, 10:43 AM