You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Bruce Lowekamp (JIRA)" <ji...@apache.org> on 2010/08/19 01:37:16 UTC
[jira] Created: (THRIFT-857) tests run by "make install" fail if
generators are disabled
tests run by "make install" fail if generators are disabled
-----------------------------------------------------------
Key: THRIFT-857
URL: https://issues.apache.org/jira/browse/THRIFT-857
Project: Thrift
Issue Type: Bug
Components: Test Suite
Affects Versions: 0.4
Environment: mac os
Reporter: Bruce Lowekamp
Priority: Minor
configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900486#action_12900486 ]
Bryan Duxbury commented on THRIFT-857:
--------------------------------------
I could be convinced to make it a warning, but in my experience, warnings get ignored.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900168#action_12900168 ]
Bryan Duxbury commented on THRIFT-857:
--------------------------------------
I would be in favor of removing the --disable-gen-* options. It seems to me like there is very little benefit to offering this feature. What's even the objective, making the compiler build faster?
Anyone else want to weigh in?
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900333#action_12900333 ]
David Reiss commented on THRIFT-857:
------------------------------------
> What's even the objective, making the compiler build faster?
I don't really remember. That's certainly one benefit. I'd be fine with getting rid of it if it is causing problems, but I'd rather figure out if we can get "make install" to work without them.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900764#action_12900764 ]
Bryan Duxbury commented on THRIFT-857:
--------------------------------------
That patch looked great, so I committed it. However, I don't think we've addressed the problem actually raised in this issue yet, so I'm going to leave it open.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
> Attachments: generator-subnamespace.patch
>
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bruce Lowekamp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bruce Lowekamp updated THRIFT-857:
----------------------------------
Attachment: check-requested-generator.patch
Ran into this problem again testing some of the new code in other issues. Wrote this to restrict the test for namespace declarations to only check the namespace if the generator has been requested on the command line. Very minimal testing, but it worked in getting the build to finish with only the cpp generator built.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
> Attachments: check-requested-generator.patch, generator-subnamespace.patch
>
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900482#action_12900482 ]
David Reiss commented on THRIFT-857:
------------------------------------
> If a lot of languages are going to be doing this, then I think we should make it some part of the overall generator spec. That way, we can eliminate the special cases.
Yeah, I actually hated that part of your patch. I've got a plan to allow generators to register the namespaces they use, but that doesn't solve the problem of compiling out generators or someone publishing a new .thrift file with (say) namespace as3 and someone else wanting to generate java code for it with an older version of thrift that doesn't support as3. Maybe we should just make it a warning instead of an error?
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bruce Lowekamp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bruce Lowekamp updated THRIFT-857:
----------------------------------
Attachment: generator-subnamespace.patch
This adds a method to check if a sub-namespace is supported by a generator, called via the generator_factory. It provides an implementation for the smalltalk generator. I'm not sure of that implementation. Looking at t_st_generator, it was referencing smalltalk.prefix and smalltalk.category, whereas t_program.h was referencing prefix and module. I went with what was in t_st_generator, but I won't claim to have looked at it enough to be sure that's right.
The patch now has the effect that a namespace declaration is rejected if the generator is not present or if the sub-namespace declaration isn't supported by a present generator.
I'm not completely sure I think a warning wouldn't be better if the generator isn't present at all.
Minimally tested, but seems to work.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
> Attachments: generator-subnamespace.patch
>
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bruce Lowekamp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900354#action_12900354 ]
Bruce Lowekamp commented on THRIFT-857:
---------------------------------------
I submitted a THRIFT-859 (with patch) that proposes reverting the check that caused this problem in the first place.
t_program.h was revised to include (everything but the last namespace[language]=namespace assignment:
void set_namespace(std::string language, std::string name_space) {
t_generator_registry::gen_map_t my_copy = t_generator_registry::get_generator_map();
t_generator_registry::gen_map_t::iterator it;
it=my_copy.find(language);
if (it == my_copy.end()) {
if (language != "smalltalk.prefix" && language != "smalltalk.package" && language != "py.twisted") {
throw "No generator named '" + language + "' could be found!";
}
}
namespaces_[language] = name_space;
}
Not only does this blow up when a generator is disabled, but it also
- prevents anyone from adding a new sub-namespace without modifying this code, as with smalltalk.prefix and smalltalk.package
- actually skips the check it's intended to do in the smalltalk case, i.e. you can declare a smalltalk.prefix namespace without having the smalltalk generator enabled
We ran into this because we had a mod to declare a separate namespace for py.twisted, which is what I'm about to put an issue in for.
Now, if it's really important to check that the generator is there for a namespace declaration, then the *right* way to do this is to take the base language name, check that there's a generator for it, then if there's a sub-language (smalltalk.package or py.twisted) call into the generator to see if it recognizes the declaration.
That seems like a lot of effort to put the support into every generator for relatively little benefit. I care a lot more about whether there's a missing namespace declaration for a language I'm trying to generate than if there's a namespace declaration for a language I'm not trying to generate.
So as part of THRIFT-859, I think we should just revert the change to t_program.h, allow any namespace declaration to be made, and let the generator report an error if it's unhappy with there not being a namespace, etc.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-857) tests run by "make install" fail if
generators are disabled
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900461#action_12900461 ]
Bryan Duxbury commented on THRIFT-857:
--------------------------------------
To take a step back, do we really want to support arbitrary sub-namespace specifications? If a lot of languages are going to be doing this, then I think we should make it some part of the overall generator spec. That way, we can eliminate the special cases.
I think that having the check is a really useful piece of functionality. It keeps you from making silly mistakes and banging your head against a wall due to a typo, which is something I think the compiler should readily support.
> tests run by "make install" fail if generators are disabled
> -----------------------------------------------------------
>
> Key: THRIFT-857
> URL: https://issues.apache.org/jira/browse/THRIFT-857
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.4
> Environment: mac os
> Reporter: Bruce Lowekamp
> Priority: Minor
>
> configuring with generators disabled (configure --disable-gen-java, for example) produces a failed build because the tests run by "make install" require the java, cpp, rb, perl, csharp, and js generators.
> I would personally favor either removing those --disble-gen options altogether, or adding a warning message in the configure --help output that disabling those generators is not recommended/will cause test failure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.