Page MenuHomeFreeBSD

depend-cleanup: Add a regex for files that moved
AcceptedPublic

Authored by andrew on Mon, Jan 20, 12:54 PM.

Details

Reviewers
jrtc27
emaste
fuz
Summary

When a file is moved or copied to a new directory the depend-cleanup
script may incorrectly remove the .depend file as the generated regex
will pass. Fix this by passing in a new regex for the arm64 files that
have the same name as a generic implementation we were previously
using.

While here add memchr.S. It has moved from a generated file to one in
the source tree with the same name.

Sponsored by: Arm Ltd

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 61801
Build 58685: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Mon, Jan 20, 1:35 PM

It would be great to have better documentation and some examples on how to add things to this script and under which precise circumstances it's needed to add something.

It would be great to have better documentation ...

Yes, absolutely. The comment at the beginning of this file lists cases where special handling is needed, although it doesn't mention the case where a file moves from one directory to another. We should have specific examples for each of these cases.

I think comments in this file are the most appropriate place for examples, but we should have a description of the make dependency problem and a reference to this file in the committer's guide or so.

Since this seems to be a saga that keeps on giving, I'm going to ask the important question to which the answer must have been no for every other commit for this, despite my comments on IRC:

Has this been properly tested? That is, have you verified:

  1. After a clean build of the last commit before any of the aarch64 SIMD commits, running this script deletes every file that intends to? (-n and -v are your friend here)
  2. After a build of the last aarch64 SIMD commit, or later, running this script does not delete any files? (ditto re -n and -v)

Until that's been done it's just going to be a game of whack-a-mole patching up every new issue that someone hits.

I built rG84de8c51d1a0fff1c65cd1ec44dd3c3a0e7904eb, i.e. the commit before the SIMD changes. After the buildworld, in a tree with no local changes the following dependencies in lib/libcrypt are removed:

Removing stale dependencies and objects for crypt-md5.c
Removing stale dependencies and objects for crypt-nthash.c
Removing stale dependencies and objects for crypt-sha256.c
Removing stale dependencies and objects for crypt-sha512.c

These are out of scope for this change and I haven't looked into why they are needed.

I updated to rG5b86888bae651e54ccc0adde0ed897ec1c1e0d45 as it was the tip of main when I tested. I ran a diff of the output of ALL_libcompats= MACHINE=arm64 MACHINE_ARCH=aarch64 ./tools/build/depend-cleanup.sh -n -v $OBJDIR both without this patch and with it. The only difference was the memchr.S dependency was cleaned.

After a buildworld the unpatched depend-cleanup.sh will try to remove the dependencies for the following in libc:

Removing stale dependencies and objects for strpbrk.c
Removing stale dependencies and objects for strcat.c
Removing stale dependencies and objects for strncat.c
Removing stale dependencies and objects for strlcat.c
Removing stale dependencies and objects for bcopy.c
Removing stale dependencies and objects for bzero.c

The only libc files the patched version tried to clean are the old generated files.

There is a separate issue where the lib32 dependencies are incorrectly cleaned that I haven't tried to fix in this patch so have excluded from the above testing.