Opened 7 years ago

Closed 7 years ago

#2698 closed defect (fixed)

src/bin/dbutil/tests fails if SHELL=/bin/csh

Reported by: fujiwara Owned by: muks
Priority: medium Milestone: Sprint-20130319
Component: tests Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Low
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

src/bin/dbutil/tests/dbutil_test.sh.in calls ../run_dbutil.sh using ${SHELL}.

${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile

${SHELL} is not controlled by BIND 10.

These '${SHELL}' should be removed or replaced by '@SHELL@'.

Subtickets

Change History (12)

comment:1 Changed 7 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

Seems straightforward.

comment:2 Changed 7 years ago by muks

See for discussion: https://lists.isc.org/pipermail/bind10-dev/2013-February/004354.html

Note that the shell invocation itself should not be removed (as suggested in the first email in that thread). Please see the full thread.

comment:3 Changed 7 years ago by shane

  • Milestone changed from Previous-Sprint-Proposed to Next-Sprint-Proposed

comment:4 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20130319

comment:5 Changed 7 years ago by muks

Please ask reporter to check if this bug fixes #2748 also.

comment:6 follow-up: Changed 7 years ago by muks

  • Owner set to UnAssigned
  • Status changed from new to reviewing

Up for review. No other references to ${SHELL} remain in the rest of the codebase.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by jinmei

Replying to muks:

Up for review. No other references to ${SHELL} remain in the rest of the codebase.

The change looks okay. The only usual question: do we need a
changelog entry?

comment:8 Changed 7 years ago by jinmei

  • Owner changed from UnAssigned to muks

comment:9 in reply to: ↑ 7 ; follow-up: Changed 7 years ago by muks

  • Owner changed from muks to jinmei

Replying to jinmei:

Replying to muks:

Up for review. No other references to ${SHELL} remain in the rest of the codebase.

The change looks okay. The only usual question: do we need a
changelog entry?

I don't feel we do for this one, as it's something in the unittests, and happened in some platform. Functionality didn't change, and though it's a bugfix, it just calls the way shell is invoked but essentially nothing changed.

In case you feel one is necessary, here would be a ChangeLog for it:

XYZ.    [bug]   	muks
	A bug in how the shell was invoked in the dbutil unittests was
        fixed.
        (Trac #2698, git ...)

comment:10 in reply to: ↑ 9 Changed 7 years ago by jinmei

Replying to muks:

Up for review. No other references to ${SHELL} remain in the rest of the codebase.

The change looks okay. The only usual question: do we need a
changelog entry?

I don't feel we do for this one, as it's something in the unittests, and happened in some platform. Functionality didn't change, and though it's a bugfix, it just calls the way shell is invoked but essentially nothing changed.

That's fine, just checked as a routine practice:-). I think the
branch can be merged then.

comment:11 Changed 7 years ago by jinmei

  • Owner changed from jinmei to muks

comment:12 Changed 7 years ago by muks

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged to master branch in commit 8d178815af6a1eda06930288f3bcb4b261e25bbd:

* f4d68d5 [2698] Fix shell invocation

Resolving as fixed. Thank you for the review Jinmei.

Note: See TracTickets for help on using tickets.