Page MenuHomePhabricator

[libcxx] Codesign test executables if necessary
ClosedPublic

Authored by vsk on Aug 20 2019, 1:57 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Aug 20 2019, 1:57 PM
vsk updated this revision to Diff 216243.Aug 20 2019, 1:58 PM
  • Attach a diff with context.
vsk updated this revision to Diff 216298.Aug 20 2019, 5:47 PM
  • Teach '%run' about code signing.
  • Introduce run.py to handle code signing in commands like 'not %run …'. A '%run' substitution that includes the codesign command cannot achieve the same thing.
ldionne added inline comments.Aug 29 2019, 7:28 AM
libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
12 ↗(On Diff #216298)

Note to self and other reviewers: I checked and I don't think you missed this change in any other test -- all the other .sh.cpp tests already use %run.

libcxx/utils/libcxx/test/config.py
1032–1037 ↗(On Diff #216298)

Just to be sure there's no subtle interaction I'm missing -- this is an entirely separate change packed into this patch, correct?

libcxx/utils/run.py
40 ↗(On Diff #216298)

Why don't we instead use sys.argv[2:], and avoid the funny delete argv[0] calls above?

vsk marked 2 inline comments as done.Aug 29 2019, 10:20 AM
vsk added inline comments.
libcxx/utils/libcxx/test/config.py
1032–1037 ↗(On Diff #216298)

Yes. I can split this out into a separate patch if it helps. It occurred to me that this was a problem when I wrote "subprocess.call(sign_cmd, env={})" in run.py, so I thought I'd fix it everywhere in one shot.

libcxx/utils/run.py
40 ↗(On Diff #216298)

Sounds good. I'll update the style in not.py as a follow-up.

vsk updated this revision to Diff 217935.Aug 29 2019, 10:32 AM
  • Address coding style issue in run.py.
ldionne accepted this revision.Sep 5 2019, 2:10 PM
This revision is now accepted and ready to land.Sep 5 2019, 2:10 PM
Closed by commit rL371126: [libcxx] Codesign test executables if necessary (authored by vedantk, committed by ). · Explain WhySep 5 2019, 2:23 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 5 2019, 2:23 PM