[CDRIVER-1337] Add configure flag for unstable MongoDB 3.4 support Created: 17/Jun/16 Updated: 08/Aug/16 Resolved: 08/Aug/16 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | Build |
| Affects Version/s: | 1.4.0 |
| Fix Version/s: | 1.4.0 |
| Type: | New Feature | Priority: | Major - P3 |
| Reporter: | A. Jesse Jiryu Davis | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Description |
|
Add a configure flag to libbson and libmongoc's Autotools and CMake build systems, something like --enable-experimental-features that is off by default, undocumented, and enables decimal128 and other MongoDB 3.4 features in the C Driver on master. Unhide these features for the 1.5 release. |
| Comments |
| Comment by A. Jesse Jiryu Davis [ 08/Aug/16 ] |
|
Closing unless anyone objects to the current solution. |
| Comment by Hannes Magnusson [ 05/Aug/16 ] |
|
I'm fine with hiding it from our configure flags. The original idea was to have it totally hidden and require manually adding it to your CFLAGS before running ./configure. That idea proved difficult because we have to export different symbols, along with various tedious details. I can't think of a way to remove it from the ./configure --help listing, while keeping it as a supported flag. Is that possible? Trying to extract an actionable item from your comment, best I can suggest is to add extra experimental disclaimer when using the flag? Since humans don't edit the mongoc-config.h, I'm not sure what difference it makes if its called __MONGOC_INTERNAL_EXPERIMENT or MONGOC_EXPERIMENTAL. If you think its worth the change in mongoc, libbson, phpc, and the outstanding c++11 PR, then.. ok. |
| Comment by David Golden [ 05/Aug/16 ] |
|
After some reflection and discussions with acm and jesse, and following the principle of "strong opinions, lightly held", I've come around to a different way of thinking:
|
| Comment by Hannes Magnusson [ 04/Aug/16 ] |
|
I don't think its a good idea. I think it needs to be deliberate decision if C++11 r3.1.0 has MongoDB 3.4 support or not. Not "maybe" and "it depends". No driver should ship production ready driver with experimental features enabled by default. But thats effectively what happens when the feature macro is kept around for 1.5, then the experimental code in C++11 r3.1.0 that was work-in-progress suddenly has hardcoded 3.4 support which isn't complete. |
| Comment by A. Jesse Jiryu Davis [ 04/Aug/16 ] |
|
Like I said, MONGODB_34_EXPERIMENTAL "is off by default in 1.4.0, In 1.5.0 it's on and you can't turn it off." It won't be removed. |
| Comment by Hannes Magnusson [ 04/Aug/16 ] |
|
I'm not sure what #1 solves? If we remove the MONGODB_34_EXPERIMENTAL their code still won't work, so we'd have to keep it around forever or remove it "in a very coordinated" way. In related note: When we originally added this I was in favor of independent flags, and still am. But it doesn't resolve the issues pointed out (which I don't think are issues). The experimental flag is meant to be removed. Drivers using it are meant to remove it. It is meant as a one off batch of things that dependent drivers can start playing with, breaking, and developing initial support. Dependent drivers CANNOT advertise availability of these features behind experimental. We have to get these features out to dependent drivers as early as we can to give them the chance to work with them with plenty of time before MongoDB 3.4. Releasing them last week of Q3 when the server ships is cruel. Once time comes for us to ship our, and dependent drivers, MongoDB 3.4 compatibility releases, the flag will be removed and dependent drivers are expected to remove the check and then announce its availability. I have no issue with the code churn. It is removal of a code block that declared it experimental and intentionally promoting it to be fully valued feature of the driver, or removing the feature completely as it didn't make the cut for MongoDB 3.4. Still checking for MONGOC_34_FEATURES or MONGOC_EXPERIMENTAL_HANDSHAKE_DATA when the feature has been removed, or dramatically changed for MongoDB 3.6 so we'd have to change the API isn't going to help you much. And if we unconditionally would set the MONGOC_34_FEATURES flag in 1.5, the features would accidentally be promoted to fully qualified features in the dependent drivers in versions they had no intentions of supporting 3.4. |
| Comment by David Golden [ 04/Aug/16 ] |
|
Bikeshed for #1: How about calling it MONGODB_34_FEATURES instead of MONGODB_34_EXPERIMENTAL. I.e. coarse-grained features rather than "experimental". It's going to be weird to see "experimental" locked on after it's not experimental anymore and this way even after depending on 1.5.0, we can just leave the flag alone. |
| Comment by A. Jesse Jiryu Davis [ 04/Aug/16 ] |
|
Hannes, would you be willing to do this? Various people have suggested that instead of a general "experimental" flag, we should have specific feature flags. david.golden convinced me in a GitHub comment. I have two proposals: Proposal one. Rename the flag MONGODB_34_EXPERIMENTAL. It's off by default in 1.4.0, In 1.5.0 it's on and you can't turn it off. Downstream drivers can #ifdef MONGODB_34_EXPERIMENTAL for now. Once they depend on 1.5.0 they can remove their ifdefs. We can continue to step forward like this in the future: MONGODB_36_EXPERIMENTAL and so on. I like proposal one because it's easy for us to search-and-replace our whole codebase, and it's easy for downstream drivers to search-and-replace theirs, instead of doing finicky work for each feature. Proposal two. Feature flags:
|
| Comment by Githook User [ 27/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 20/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 20/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by A. Jesse Jiryu Davis [ 20/Jul/16 ] |
|
Need to disable BSON_ITER_HOLDS_DECIMAL128, VISIT_DECIMAL128. |
| Comment by Githook User [ 18/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 18/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 14/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 07/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 07/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: I'd thought specifying two symbol files with CMake's add_library would |
| Comment by Githook User [ 06/Jul/16 ] |
|
Author: {u'username': u'bjori', u'name': u'Hannes Magnusson', u'email': u'bjori@php.net'}Message: |
| Comment by Githook User [ 06/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Githook User [ 06/Jul/16 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |