Closed #619 via 62d2a09b3a37b22b70bfec78da162ca55224c072.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/619#event-1896758749
--
openzfs:
that cmdlines get
processed as expected).
From: Brian Behlendorf
Reply: openzfs-developer
Date: September 25, 2018 at 1:32:51 PM
To: openzfs/openzfs
Cc: Subscribed
Subject: [developer] Re: [openzfs/openzfs] 9466 add JSON output support to
channel programs (#619)
I've bumped on this a few
jwk404 approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/619#pullrequestreview-159035992
--
openzfs:
I was going to make a post similar to what @behlendorf posted, but I deleted
what I had typed when I realized that we might end up breaking a bunch of
people's scripts who are using non `POSIXLY_CORRECT` option formats. I don't
really have a good answer for how to do that.
--
You are
I've bumped on this a few times now. This `getopt(3)` behavior is controlled
by the `POSIXLY_CORRECT` environment variable. Setting this will get you the
illumos behavior, but it is not the default on Linux. See the full discussion
in the man page.
I'm sure we can fix this but I have not looked into what it would take. I do
agree that it would be really nice to have them behave the same...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
I see, linux allows `zfs program pool -j ...` but illumos requires the flags to
be before the positional arguments (`zfs program -j pool ...`). Do you know if
that's something that we could bring into alignment across the platforms?
Seems like it would help with scriptability.
--
You are
Looks like illumos differs with Linux in how we do cli argument parsing. I've
switched the flags to `zfs program` in the the test (`zfs_program_json.ksh`)
and it's now passing.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on
@alek-p I reran the tests last night, and I see a zfs-test failure:
```
Test: /opt/zfs-tests/tests/functional/cli_root/zfs_program/zfs_program_json
(run as root) [00:01] [FAIL]
20:57:00.06 ASSERTION: Channel programs output valid JSON
20:57:00.11 SUCCESS: zfs create testpool/zcp-json
20:57:00.15
alek-p commented on this pull request.
> @@ -0,0 +1,30 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
alek-p commented on this pull request.
> + end
+
+ args = ...
+
+ argv = args["argv"]
+
+ list_recursive(argv[1], argv[2])
+
+ results = {}
+ results["succeeded"] = succeeded
+ results["failed"] = failed
+ return results
+EOF
+
+# 1. Compare JSON
alek-p commented on this pull request.
> +# at http://www.illumos.org/license/CDDL.
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2018 Datto Inc.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+#
+# STRATEGY:
+# 1. Compare JSON output formatting for a channel program to
alek-p commented on this pull request.
> @@ -0,0 +1,30 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
jwk404 requested changes on this pull request.
> + end
+
+ args = ...
+
+ argv = args["argv"]
+
+ list_recursive(argv[1], argv[2])
+
+ results = {}
+ results["succeeded"] = succeeded
+ results["failed"] = failed
+ return results
+EOF
+
+# 1. Compare
richardelling commented on this pull request.
> @@ -0,0 +1,30 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the
ahrens approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/619#pullrequestreview-120752037
--
openzfs:
sdimitro commented on this pull request.
> +# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source. A copy is
alek-p commented on this pull request.
> +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered
alek-p commented on this pull request.
> +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered
alek-p commented on this pull request.
> +# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source. A copy is
sdimitro approved this pull request.
Can you add an entry in `usr/src/pkg/manifests/system-test-zfstest.mf` for the
new test that you added?
Things look good to me overall since I've had a sneak peek of the ZoL review.
I'd have @jwk404 take a look at the new test case though just in case.
>
21 matches
Mail list logo
