Merge lp:~zorba-coders/zorba/1025564 into lp:zorba

Proposed by Chris Hillery on 2012-09-19
Status: Merged
Approved by: Chris Hillery on 2013-05-01
Approved revision: 11339
Merged at revision: 11421
Proposed branch: lp:~zorba-coders/zorba/1025564
Merge into: lp:zorba
Diff against target: 194 lines (+50/-17)
5 files modified
ChangeLog (+2/-0)
bin/CMakeLists.txt (+14/-1)
bin/test/shebang.xq.in (+2/-0)
bin/zorbacmdproperties.cpp (+10/-11)
bin/zorbacmdproperties_base.h (+22/-5)
To merge this branch: bzr merge lp:~zorba-coders/zorba/1025564
Reviewer Review Type Date Requested Status
Matthias Brantner 2012-09-19 Approve on 2013-04-30
Chris Hillery Approve on 2013-04-30
Review via email: mp+125256@code.launchpad.net

This proposal supersedes a proposal from 2012-08-16.

Commit message

Added changes for deprecating -f option.

To post a comment you must log in.

Works great. Some minor issues:

- the --help documentation should mention that the argument is deprecated
- also, the --help documentation should mention the syntax to pass the file
- please respect coding guidelines. For example,
if(qBeforeF()){ => if (qBeforeF())
{
- remove commented out code

review: Needs Fixing
Chris Hillery (ceejatec) wrote :

Agree about the --help doc; if I get a little time later I might put in a suggested change myself.

Also agree about not leaving commented-out code.

To be fair to Luis, the coding guidelines do not mention anything about brace positioning or spaces after "if", etc.

Chris Hillery (ceejatec) wrote :

Also, add a mention to the option changes to the ChangeLog.

review: Needs Fixing
Luis Rodriguez Gonzalez (kuraru) wrote :

Comments fixed, please re-review.

Chris Hillery (ceejatec) wrote :

I'm not sure why Launchpad is showing hundreds of changes on this branch; something odd when merging from the trunk, I assume. But the diff looks good, so if it merges OK, I'm happy.

review: Approve

> I'm not sure why Launchpad is showing hundreds of changes on this branch;
> something odd when merging from the trunk, I assume. But the diff looks good,
> so if it merges OK, I'm happy.

Since most of the changes are in bin folder, that has not been changed in a while now, just to be 100% sure that everything is OK one could:
- create a new branch of lp:zorba locally
- merge the proposed changes in this branch and propose it for merging (be sure to give it a unique name when pushing the changes to LP)
- delete this merge proposal and related branch

What happened to "Fixed bug #866958 (Parsing error not explicit enough)" in the ChangeLog?

review: Needs Information
Chris Hillery (ceejatec) wrote :

That bug was never part of this proposal. I think it's showing up in the did here due to my merge from the trunk. The Changelog had a number of things get rearranged which made conflict resolution hard. That bug is in fact fixed so I assume it should be in the Changelog.

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :
Zorba Build Bot (zorba-buildbot) wrote :

The attempt to merge lp:~zorba-coders/zorba/1025564 into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275 (message):
  Validation queue job 1025564-2013-04-30T22-40-03.37Z is finished. The
  final status was:

  1 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

Zorba Build Bot (zorba-buildbot) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Zorba Build Bot (zorba-buildbot) wrote :
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job 1025564-2013-05-01T03-32-07.037Z is finished. The final status was:

All tests succeeded!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-04-30 23:54:17 +0000
3+++ ChangeLog 2013-05-01 03:16:28 +0000
4@@ -94,6 +94,7 @@
5 * Fixed bug in computing the static type of a self-axis path step.
6 * Fixed bug #867068: (Incorrect usage of XQDY0027)
7 * Fixed bug: jsoniq constructor should not be const-folded
8+ * Fixed bug #866958 (Parsing error not explicit enough)
9 * Fixed bug #1152834 (bug in computing the scripting kind of array and object
10 constructor exprs)
11 * Fixed bug #1125444 (input group-by exprs were not treated by index-flwor_vars()
12@@ -101,6 +102,7 @@
13 * Raise XQST0046, if needed, from namespace declaration attribute inside
14 direct element constructor.
15 * Fixed bug #1023362 (xsi:type attribute ignored during validation)
16+ * Fixed bug #1025564 (Deprecate -f argument to zorbacmd)
17 * Fixed bug #1082740 (support for INF and -INF values in fn:subsequence function).
18
19
20
21=== modified file 'bin/CMakeLists.txt'
22--- bin/CMakeLists.txt 2013-02-07 17:24:36 +0000
23+++ bin/CMakeLists.txt 2013-05-01 03:16:28 +0000
24@@ -58,6 +58,12 @@
25 ENDIF(UNIX)
26
27 # test the basic features of the command line tool
28+ZORBA_ADD_TEST(bin/zorba_f1 zorbacmd blub.xq)
29+ZORBA_SET_TEST_PROPERTY(bin/zorba_f1 PASS_REGULAR_EXPRESSION ".*file {blub.xq} not found or not readable.*")
30+
31+ZORBA_ADD_TEST(bin/zorba_f2 zorbacmd "${CMAKE_CURRENT_SOURCE_DIR}/test/option.xq" --option "{http://www.zorba-xquery.com}option=highly_unique")
32+ZORBA_SET_TEST_PROPERTY(bin/zorba_f2 PASS_REGULAR_EXPRESSION "highly_unique")
33+
34 ZORBA_ADD_TEST(bin/zorba1 zorbacmd --omit-xml-declaration -q "1+1")
35 ZORBA_SET_TEST_PROPERTY(bin/zorba1 PASS_REGULAR_EXPRESSION "2")
36
37@@ -71,7 +77,14 @@
38 ZORBA_SET_TEST_PROPERTY(bin/zorba4 PASS_REGULAR_EXPRESSION "(.*<child>child1</child>)*")
39
40 ZORBA_ADD_TEST(bin/zorba5 zorbacmd -q -f blub.xq)
41-ZORBA_SET_TEST_PROPERTY(bin/zorba5 PASS_REGULAR_EXPRESSION ".*Extra arguments found on command line.*")
42+ZORBA_SET_TEST_PROPERTY(bin/zorba5 PASS_REGULAR_EXPRESSION ".*No queries submitted.*")
43+
44+IF (UNIX)
45+ CONFIGURE_FILE ("${CMAKE_CURRENT_SOURCE_DIR}/test/shebang.xq.in"
46+ "${CMAKE_CURRENT_BINARY_DIR}/shebang.xq" @ONLY)
47+ ADD_TEST (bin/shebang "${CMAKE_CURRENT_BINARY_DIR}/shebang.xq")
48+ SET_TESTS_PROPERTIES (bin/shebang PROPERTIES PASS_REGULAR_EXPRESSION "shebang success")
49+ENDIF (UNIX)
50
51 # test the compile-only option of the command line tool which is used for semantic checking in eclipse
52 ZORBA_ADD_TEST(bin/zorba_compilechk1 zorbacmd -q "${CMAKE_CURRENT_SOURCE_DIR}/test/mymod.xq" -f -l)
53
54=== added file 'bin/test/shebang.xq.in'
55--- bin/test/shebang.xq.in 1970-01-01 00:00:00 +0000
56+++ bin/test/shebang.xq.in 2013-05-01 03:16:28 +0000
57@@ -0,0 +1,2 @@
58+#!@ZORBA_EXE@
59+"shebang success"
60
61=== modified file 'bin/zorbacmdproperties.cpp'
62--- bin/zorbacmdproperties.cpp 2013-02-07 17:24:36 +0000
63+++ bin/zorbacmdproperties.cpp 2013-05-01 03:16:28 +0000
64@@ -23,22 +23,21 @@
65
66 std::string ZorbaCMDProperties::check_args () {
67
68- if(queriesOrFilesBegin() == queriesOrFilesEnd()) {
69- return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba -f -q file.xq execute a query from a file \n Use -h for help.";
70+ if(queriesOrFilesBegin() == queriesOrFilesEnd())
71+ {
72+ return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba file.xq execute a query from a file \n Use -h for help.";
73 }
74
75- if (getPositionalArgs ().size () != 0) {
76- QueriesOrFiles_t::const_iterator lIter;
77- for (lIter = queriesOrFilesBegin(); lIter != queriesOrFilesEnd();
78- ++lIter)
79- {
80- if (*lIter == "-f")
81- return "Extra arguments found on command line. Possible reason: misplaced -f option (try '-f -q filename' instead of '-q -f filename'). Use -h for help.";
82- }
83-
84+ if(unknownOption())
85+ {
86 return "Extra arguments found on command line. Use -h for help.";
87 }
88
89+ if(queriesOrFilesBegin() == queriesOrFilesEnd())
90+ {
91+ return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba file.xq execute a query from a file \n Use -h for help.";
92+ }
93+
94 if ( theBoundarySpace.size() != 0 )
95 {
96 if ( ! (theBoundarySpace.compare("strip") == 0 || theBoundarySpace.compare("preserve") == 0 ))
97
98=== modified file 'bin/zorbacmdproperties_base.h'
99--- bin/zorbacmdproperties_base.h 2013-02-07 17:24:36 +0000
100+++ bin/zorbacmdproperties_base.h 2013-05-01 03:16:28 +0000
101@@ -94,6 +94,9 @@
102 bool theSerializePlan;
103 bool theSavePlan;
104 bool theLoadPlan;
105+ bool theFparm;
106+ bool theQBeforeF;
107+ bool theUnknownOption;
108
109 void initialize ()
110 {
111@@ -106,7 +109,7 @@
112 theByteOrderMark = false;
113 theOmitXmlDeclaration = false;
114 theMultiple = 1;
115- theAsFiles = false;
116+ theAsFiles = true;
117 theOptimizationLevel = "O1";
118 theLibModule = false;
119 theParseOnly = false;
120@@ -121,6 +124,9 @@
121 theSerializePlan = false;
122 theSavePlan = false;
123 theLoadPlan = false;
124+ theFparm = false;
125+ theQBeforeF = false;
126+ theUnknownOption = false;
127 }
128
129 public:
130@@ -167,6 +173,8 @@
131 const bool& serializePlan () const { return theSerializePlan; }
132 const bool& loadPlan () const { return theLoadPlan; }
133 const bool& savePlan () const { return theSavePlan; }
134+ const bool& qBeforeF () const { return theQBeforeF; }
135+ const bool& unknownOption() const { return theUnknownOption; }
136
137 std::string load_argv (int argc, const char **argv)
138 {
139@@ -279,6 +287,13 @@
140 else if (strcmp (*argv, "--query") == 0 || strncmp (*argv, "-q", 2) == 0)
141 {
142 int d = 2;
143+ if(theFparm == false)
144+ theAsFiles = false;
145+ if(!strncmp(*(argv+1), "-f", 2))
146+ {
147+ theQBeforeF = true; // is it "-q -f <filename>" perhaps?
148+ break; // stop functionality here
149+ }
150 if ((*argv) [1] == '-' || (*argv) [2] == '\0') { d = 0; ++argv; }
151 if (*argv == NULL)
152 {
153@@ -288,6 +303,7 @@
154 }
155 else if (strcmp (*argv, "--as-files") == 0 || strncmp (*argv, "-f", 2) == 0)
156 {
157+ theFparm = true;
158 theAsFiles = true;
159 }
160 else if (strcmp (*argv, "--external-variable") == 0 || strncmp (*argv, "-e", 2) == 0)
161@@ -449,12 +465,13 @@
162 }
163 else if ((*argv) [0] == '-')
164 {
165- result = "unknown command line option "; result += *argv; break;
166+ result = "unknown command line option "; result += *argv;
167+ theUnknownOption = true; break;
168 }
169 else
170 {
171+ init_val(*argv, theQueriesOrFiles, 0);
172 copy_args (argv);
173- break;
174 }
175 }
176
177@@ -481,7 +498,7 @@
178 "--ordering-mode\nSet the ordering mode ('ordered' or 'unordered') in the static context.\n\n"
179 "--multiple, -m\nExecute the given queries multiple times.\n\n"
180 "--query, -q\nQuery test or file URI (file://...)\n\n"
181- "--as-files, -f\nTreat all -q arguments as file paths instead of URIs or inline queries.\n\n"
182+ "--as-files, -f\nTreat all -q arguments as file paths instead of URIs or inline queries. This option is deprecated and will be defaulted to true in the future, so any entry in the command line is going to be treated as files.\n\n"
183 "--external-variable, -e\nProvide the value for a variable given a file (name=file) or a value (name:=value)\n\n"
184 "--context-item\nSet the context item to the XML document in a given file.\n\n"
185 "--optimization-level\nOptimization level for the query compiler (O0, O1 or O2 - default: O1)\n\n"
186@@ -504,7 +521,7 @@
187 "--thesaurus\nMapping specifying a thesaurus URI to another.\n\n"
188 "--serialize-plan, -s\nSerialize and then load the query execution plan.\n\n"
189 "--compile-plan,\nDo not execute the query; just compile it and save the execution plan in the file specified with the -o option.\n\n"
190- "--execute-plan\nDo not compile the query; instead load the execution plan from the file specified by the -f -q options, and execute the loaded plan.\n\n"
191+ "--execute-plan\nDo not compile the query; instead load the execution plan from the file specified by the -f -q options (or by any file specified without any other argument), and execute the loaded plan.\n\n"
192 "--disable-http-resolution\nDo not use HTTP to resolve URIs\n\n"
193 ;
194 }

Subscribers

People subscribed via source and target branches