Bug 191052 - atf-sh integration tests broken after r267181
Summary: atf-sh integration tests broken after r267181
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Julio Merino,+1 347 694 0576,New York City
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-15 04:59 UTC by Enji Cooper
Modified: 2014-09-09 04:01 UTC (History)
2 users (show)

See Also:


Attachments
Prefix all references to atf-sh with /usr/bin/env so the sed operation fills in /usr/libexec/atf-sh (1.40 KB, application/mbox)
2014-06-15 04:59 UTC, Enji Cooper
no flags Details
Fix path to atf-sh in test program (2.26 KB, patch)
2014-06-27 16:22 UTC, Julio Merino,+1 347 694 0576,New York City
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enji Cooper freebsd_committer freebsd_triage 2014-06-15 04:59:41 UTC
Created attachment 143796 [details]
Prefix all references to atf-sh with /usr/bin/env so the sed operation fills in /usr/libexec/atf-sh

The atf-sh integration tests are broken after r267181 because the /usr/sbin is always expected to be in path whereas /usr/libexec isn't by design.

The attached patch fixes the issue.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2014-06-15 05:04:20 UTC
Over to committer in question.
Comment 2 Julio Merino,+1 347 694 0576,New York City freebsd_committer freebsd_triage 2014-06-27 15:55:41 UTC
The reason I added the sed magic to the Makefile was to avoid touching the files in contrib.  I'd rather do the same here if possible, though it's proving to be tricky...
Comment 3 Julio Merino,+1 347 694 0576,New York City freebsd_committer freebsd_triage 2014-06-27 16:22:12 UTC
Created attachment 144195 [details]
Fix path to atf-sh in test program

Here comes an attempt to fix this.  I'm changing all calls to atf_check to add /usr/libexec to the PATH.  To do this, and to make upgrading transparent, I need to make integration_test depend on the Makefile... which in turn required some changes to *.test.mk.  (The changes to the latter are hackish; not sure how to make nicer.)

Testing the fix and will submit if things work fine.
Comment 4 Enji Cooper freebsd_committer freebsd_triage 2014-07-08 00:31:53 UTC
(In reply to Julio Merino from comment #3)
> Created attachment 144195 [details]
> Fix path to atf-sh in test program
> 
> Here comes an attempt to fix this.  I'm changing all calls to atf_check to
> add /usr/libexec to the PATH.  To do this, and to make upgrading
> transparent, I need to make integration_test depend on the Makefile... which
> in turn required some changes to *.test.mk.  (The changes to the latter are
> hackish; not sure how to make nicer.)
> 
> Testing the fix and will submit if things work fine.

The proposed patch assumes that the extensions for script sources is always .sh -- that might not always be the case :/.
Comment 5 Julio Merino,+1 347 694 0576,New York City freebsd_committer freebsd_triage 2014-07-08 20:27:11 UTC
I know, but I have no better solution other than replacing the patterns to :N*Makefile* (which I have done locally).  Not perfect, sure, but I think it's a good-enough compromise.  If this ever becomes a problem we can revisit.
Comment 6 Enji Cooper freebsd_committer freebsd_triage 2014-07-08 20:40:06 UTC
(In reply to Julio Merino from comment #5)
> I know, but I have no better solution other than replacing the patterns to
> :N*Makefile* (which I have done locally).  Not perfect, sure, but I think
> it's a good-enough compromise.  If this ever becomes a problem we can
> revisit.

This caveat needs to be documented somewhere outside the Makefile snippet -- otherwise it's a bug waiting to be filed.
Comment 7 commit-hook freebsd_committer freebsd_triage 2014-07-09 00:56:02 UTC
A commit references this bug:

Author: jmmv
Date: Wed Jul  9 00:55:51 UTC 2014
New revision: 268445
URL: http://svnweb.freebsd.org/changeset/base/268445

Log:
  Fix atf-sh's integration_test

  With the move of atf-sh into /usr/libexec in r267181, some of the
  tests in the integration_test program broke because they could not
  execute atf-sh from the path any longer.

  This slipped through because I do have a local atf installation in
  my home directory that appears in my path, hence the tests could
  still execute my own version.

  Fix this by forcing /usr/libexec to appear at the beginning of the
  path when attempting to execute atf-sh.

  To make upgrading easy (and to avoid an unnecessary entry in UPDATING),
  make integration_test depend on the Makefile so that a rebuild of the
  shell script is triggered.  This requires a hack in the *.test.mk files
  to ensure the Makefile is not treated as a source to the generated
  program.  Ugly, I know, but I don't have a better way of doing this at
  the moment.  Will think of one once I address the TODO in the *.test.mk
  files that suggests generalizing the file generation functionality.

  PR:		191052
  Reviewed by:	Garrett Cooper

Changes:
  head/libexec/atf/atf-sh/tests/Makefile
  head/share/mk/atf.test.mk
  head/share/mk/plain.test.mk
  head/share/mk/tap.test.mk
Comment 8 commit-hook freebsd_committer freebsd_triage 2014-09-09 04:01:18 UTC
A commit references this bug:

Author: ngie
Date: Tue Sep  9 04:00:33 UTC 2014
New revision: 271298
URL: http://svnweb.freebsd.org/changeset/base/271298

Log:
  MFC r267176, r267181, r268445 (ATF-related commits):

  Phabric: https://reviews.freebsd.org/D706
  Approved by: rpaulo (mentor)
  Approved by: re (gjb)
  Reviewed by: jmmv
  Sponsored by: EMC / Isilon Storage Division

  r267176:

   Add the *_TESTS_SH_SED_* functionality to atf.test.mk.

   This exists already in plain.test.mk and tap.test.mk and should have been
   added to atf.test.mk too when the feature was first introduced.

   (It is probably time to address the related TODOs but I will do that
   separately.)

  r267181:

   Move atf-sh from /usr/bin/ to /usr/libexec/

   In r266650, we made libatf-c and libatf-c++ private libraries so that no
   components outside of the source tree could unintendedly depend on them.

   This change does the same for the "atf-sh library" by moving the atf-sh
   interpreter from its public location in /usr/bin/ to the private location
   in /usr/libexec/.  Our build system will ensure that our own test programs
   use the right binary, but users won't be able to depend on atf-sh by
   "mistake".

   Committing this now to ride the UPDATING notice added with r267172 today.

  r268445:

   Fix atf-sh's integration_test

   With the move of atf-sh into /usr/libexec in r267181, some of the
   tests in the integration_test program broke because they could not
   execute atf-sh from the path any longer.

   This slipped through because I do have a local atf installation in
   my home directory that appears in my path, hence the tests could
   still execute my own version.

   Fix this by forcing /usr/libexec to appear at the beginning of the
   path when attempting to execute atf-sh.

   To make upgrading easy (and to avoid an unnecessary entry in UPDATING),
   make integration_test depend on the Makefile so that a rebuild of the
   shell script is triggered.  This requires a hack in the *.test.mk files
   to ensure the Makefile is not treated as a source to the generated
   program.  Ugly, I know, but I don't have a better way of doing this at
   the moment.  Will think of one once I address the TODO in the *.test.mk
   files that suggests generalizing the file generation functionality.

   PR:		191052
   Reviewed by:	Garrett Cooper

Changes:
_U  stable/10/
  stable/10/UPDATING
  stable/10/etc/mtree/BSD.tests.dist
  stable/10/libexec/atf/Makefile
  stable/10/libexec/atf/Makefile.inc
  stable/10/libexec/atf/atf-check/Makefile
  stable/10/libexec/atf/atf-sh/
  stable/10/libexec/atf/atf-sh/Makefile
  stable/10/share/mk/atf.test.mk
  stable/10/share/mk/plain.test.mk
  stable/10/share/mk/tap.test.mk
  stable/10/tools/build/mk/OptionalObsoleteFiles.inc
  stable/10/usr.bin/Makefile