New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: sed on FreeBSD requires "-i extension", so use gsed #13903

Merged
merged 1 commit into from Mar 23, 2017

Conversation

Projects
None yet
2 participants
@wjwithagen
Contributor

wjwithagen commented Mar 9, 2017

  • FreeBSD sed wants a backup extention after the -i option
    or otherwise it starts eating the next option

Signed-off-by: Willem Jan Withagen wjw@digiware.nl

@@ -71,7 +71,12 @@ function TEST_classes() {
#
ceph osd getcrushmap > $dir/map || return 1
crushtool -d $dir/map -o $dir/map.txt || return 1
sed -i \
if [ `uname` = FreeBSD ]; then

This comment has been minimized.

@tchaikov

tchaikov Mar 10, 2017

Contributor

how about src/test/mon/osd-crush.sh , src/test/test_pool_create.sh and src/test/test_crush_bucket.sh, maybe we can define a function in qa/workunits/ceph-helpers.sh to normalize the difference between FreeBSD sed and GNU sed?

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 10, 2017

@tchaikov
It is certainly wobbly argument passing on behalf of FreeBSD.
And using more than one -e set is a way to trigger.
But I'll look into "generalizing" this.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 15, 2017

FreeBSD sed

sed [-Ealn] [-e command] [-f command_file] [-i extension] [file ...]

GNU sed

-i[SUFFIX], --in-place[=SUFFIX]

@wjwithagen i don't think it's triggered by -e, but yeah. we need to generalize this. because otherwise we need to patch multiple test script in the very same way.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 15, 2017

@tchaikov
Sorry, but I've been rather busy with other things lately.
But once the dust settles here, I try and fix this thru qa/workunits/ceph-helpers.sh

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 19, 2017

@tchaikov
I'm looking into ceph-helpers.sh but it would be the first "repair" function this way.
Only thing fixing there is the xmlstarlet <>xml compatibility?

Are you sure you'd like to do it this way?
ATM crush-classes.sh is the only where it does not work as expected.
So for the time being I'd keep this patch, and I'll look into the FreeBSD code of sed and/or see if we can start using the GNU version of sed.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 19, 2017

@wjwithagen

take https://github.com/ceph/ceph/blob/master/src/test/mon/osd-crush.sh#L149 as an example, are you sure FreeBSD sed works as expected? i believe it will take 's/ruleset 0/ruleset 3/' as the option of the -i argument.

test: use gsed on FreeBSD for inplace editting
 - FreeBSD sed(1) requires a extension on -i
   so replace the usuage with GNU sed: gsed

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

@wjwithagen wjwithagen changed the title from test/crush/crush-classes.sh: sed on FreeBSD requires "-i '''" to test: sed on FreeBSD requires "-i extension", so use gsed Mar 19, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 19, 2017

@tchaikov
I chose the easier way out: use GNU sed

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 19, 2017

Are you sure you'd like to do it this way?

@wjwithagen i don't really like this way. and yeah! i like the "gsed" approach more.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 19, 2017

@tchaikov
Jenkins seems happy also.
So you might want to run it thru QA?

@tchaikov tchaikov self-assigned this Mar 20, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 20, 2017

@wjwithagen will do.

@tchaikov tchaikov merged commit 1a9eae1 into ceph:master Mar 23, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@wjwithagen wjwithagen deleted the wjwithagen:wip-wjw-run-classes-sed branch Mar 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment