Project

General

Profile

Actions

Feature #14105

closed

ar -s could work on its own

Added by Robert Mustacchi 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Category:
cmd - userland programs
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

A large number of rust crates build processes invoke ar -s on their own. The purpose of this is to rebuild the symbol table index. This is the equivalent of ranlib in theory, except that in the face of a stripped binary the ar symbol table / index will be regenerated. The ability to use -s without any other flags is common in GNU ar, but for us requires the use of -t. To facilitate making it easier to build crates and to give a path to regenerate the index without printing everything out, we should allow ar -s to be specified without -t.

This basically turns into doing all the same work that ar -st would do, but without actually printing anything out. This will support the -v and -V options like other here.

To test this, I took an archive file, stripped it, and regenerated things with an old ar -st and a new ar -s to compare what happened. The symbol table indexes were identical in the end. I also used the other options -v and -V to make sure they worked as expected. There'll also be some use of various rust crates that hit this checked before RTIing this.

Actions #1

Updated by Robert Mustacchi 4 months ago

  • Gerrit CR set to 1723
Actions #2

Updated by Robert Mustacchi 4 months ago

In addition to this, I also did an illumos build and then ONU'd onto bits with this ar and did another build. wsdiff indicated no change here. We also took a rust crate or two that we were hitting this with and made sure that it worked with the new ar in place and the crates passed all their tests.

Actions #3

Updated by Robert Mustacchi 3 months ago

So, this has stumbled over some issues in the field. In particular, Dominik found a case where the openjdk build broke here. I'm sure there are others lurking. The issue is simple. The following no longer works if the archive file doesn't exist:

$ ar -crs foo.a test.o
ar: archive foo.a not found

This is unfortunate because the point of -c is to create it. While the fix for this is simple, I'd like to back this out so I can do two additional things that in hindsight I should have done initially:

1. Write a series of regression tests for basic behavior of ar here allowing us to go through various flag combinations in a before and after, which honestly I should have done.
2. Go through and do full builds of all the various third-party software repos as a better regression test.

Actions #4

Updated by Andy Fiddaman 3 months ago

Also, while you're working on this, the new usage message line was not properly indented:

usage: ar -d[-SvV] archive file ...
       ar -m[-abiSvV] [posname] archive file ...
       ar -p[-vV][-sS] archive [file ...]
       ar -q[-cuvSV] [-abi] [posname] [file ...]
       ar -r[-cuvSV] [-abi] [posname] [file ...]
       ar -s[-vV] archive
                       ar -t[-vV][-sS] archive [file ...]
       ar -x[-vV][-sSCT] archive [file ...]
Actions #5

Updated by Electric Monk 3 months ago

  • Status changed from New to Closed
  • % Done changed from 60 to 100

git commit 5a493f9b6da15815fc84f6f10e1a800f0dc59813

commit  5a493f9b6da15815fc84f6f10e1a800f0dc59813
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-10-14T04:04:14.000Z

    14105 ar -s could work on its own
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Reviewed by: Klaus Ziegler <klausz@haus-gisela.de>
    Reviewed by: Patrick Mooney <pmooney@pfmooney.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions #6

Updated by Electric Monk 3 months ago

git commit c5ef4e1ed46906c916eadf221255aa714ef9d98e

commit  c5ef4e1ed46906c916eadf221255aa714ef9d98e
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-10-17T14:58:19.000Z

    backout 14105: breaks -crs, opendjk

Actions #7

Updated by Robert Mustacchi 3 months ago

The issue with the original integration was that it didn't properly account for the fact that -rs was used, which led to an error. This was due to bad logic in how we checked whether -s was used. To make sure this didn't regress a second time, I've done the following:

  • Used #14213 to write a broader test suite and added items specific to this and other pre-existing usage of -s.
  • Dominik Hassler kindly built all of OmniOS's base/core and extra packages with the updated ar. This was what originally triggered this and we found that now there were no regressions unlike previously.
  • Built illumos-gate with this ar present
  • Did a wsdiff and verified that it was clean

The following is the util-test test suite run that drives this:

rm@iliad:~$ pfexec /opt/util-tests/bin/utiltest 
Test: /opt/util-tests/tests/allowed-ips (run as root)             [00:01] [PASS]
Test: /opt/util-tests/tests/ar/artest (run as root)               [00:02] [PASS]
Test: /opt/util-tests/tests/chown_test (run as root)              [00:01] [PASS]
Test: /opt/util-tests/tests/date_test (run as root)               [00:00] [PASS]
Test: /opt/util-tests/tests/find/findtest (run as root)           [00:00] [PASS]
Test: /opt/util-tests/tests/grep_test (run as root)               [00:03] [PASS]
Test: /opt/util-tests/tests/head/head_test (run as root)          [00:00] [PASS]
Test: /opt/util-tests/tests/libjedec_test (run as root)           [00:00] [PASS]
Test: /opt/util-tests/tests/libsff/libsff (run as root)           [00:00] [PASS]
Test: /opt/util-tests/tests/make_test (run as root)               [00:01] [PASS]
Test: /opt/util-tests/tests/mdb/mdbtest (run as root)             [00:00] [PASS]
Test: /opt/util-tests/tests/mergeq/mqt (run as root)              [00:01] [PASS]
Test: /opt/util-tests/tests/mergeq/wqt (run as root)              [00:00] [PASS]
Test: /opt/util-tests/tests/pcidbtest (run as root)               [00:02] [PASS]
Test: /opt/util-tests/tests/pcieadm-priv (run as root)            [00:02] [PASS]
Test: /opt/util-tests/tests/pcieadmtest (run as root)             [00:05] [PASS]
Test: /opt/util-tests/tests/printf_test (run as root)             [00:00] [PASS]
Test: /opt/util-tests/tests/set-linkprop (run as root)            [00:00] [PASS]
Test: /opt/util-tests/tests/sleep/sleeptest (run as root)         [00:43] [PASS]
Test: /opt/util-tests/tests/smbios (run as root)                  [00:00] [PASS]
Test: /opt/util-tests/tests/svr4pkg_test (run as root)            [00:00] [PASS]
Test: /opt/util-tests/tests/xargs_test (run as root)              [00:00] [PASS]
Test: /opt/util-tests/tests/awk/runtests.sh (run as nobody)       [02:33] [PASS]
Test: /opt/util-tests/tests/ctf/precheck (run as root)            [00:00] [PASS]
Test: /opt/util-tests/tests/ctf/ctftest (run as root)             [00:10] [PASS]
Test: /opt/util-tests/tests/demangle/afl-fast (run as root)       [00:01] [PASS]
Test: /opt/util-tests/tests/demangle/gcc-libstdc++ (run as root)  [00:00] [PASS]
Test: /opt/util-tests/tests/demangle/llvm-stdcxxabi (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libcustr/custr_remove (run as root)   [00:00] [PASS]
Test: /opt/util-tests/tests/libcustr/custr_trunc (run as root)    [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_00_blank (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_01_boolean (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_02_numbers (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_03_empty_arrays (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_04_number_arrays (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_05_strings (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_06_nested (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/libnvpair_json/json_07_nested_arrays (run as root) [00:00] [PASS]
Test: /opt/util-tests/tests/sed/sed_addr (run as root)            [00:00] [PASS]
Test: /opt/util-tests/tests/sed/multi_test (run as root)          [00:01] [PASS]

Results Summary
PASS      40

Running Time:   00:03:56
Percent passed: 100.0%
Log directory:  /var/tmp/test_results/20211111T063759
Actions #8

Updated by Electric Monk 3 months ago

git commit fb25420ba8dbfa4c292d42c87555eee97a474854

commit  fb25420ba8dbfa4c292d42c87555eee97a474854
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-11-11T19:19:33.000Z

    14105 ar -s could work on its own
    14213 Want basic ar test suite
    14212 ar cra and crb don't work
    14214 ar usage message needs updating for -q
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Reviewed by: Patrick Mooney <pmooney@pfmooney.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF