Skip to content
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

Build failure with Protobuf 3.7.0 and unspecified future version #460

Closed
Arfrever opened this issue Mar 15, 2019 · 3 comments
Closed

Build failure with Protobuf 3.7.0 and unspecified future version #460

Arfrever opened this issue Mar 15, 2019 · 3 comments

Comments

@Arfrever
Copy link

Arfrever commented Mar 15, 2019

Mozc fails to build with Protobuf 3.7.0:

FAILED: obj/dictionary/user_dictionary.user_dictionary.o 
/usr/bin/x86_64-pc-linux-gnu-g++ -MMD -MF obj/dictionary/user_dictionary.user_dictionary.o.d -DOS_LINUX -DMOZC_BUILD -DCHANNEL_DEV -DENABLE_GTK_RENDERER -DNDEBUG -DQT_NO_DEBUG -DNO_LOGGING -DIGNORE_HELP_FLAG -DIGNORE_INVALID_FLAG -I/var/tmp/portage/app-i18n/mozc-9999/work/mozc-9999/src -Igen -Igen/proto_out -Wall -Wno-char-subscripts -Wno-sign-compare -Wno-deprecated-declarations -Wwrite-strings -Wno-unknown-warning-option -Wno-inconsistent-missing-override -fPIC -fno-exceptions -fmessage-length=0 -fno-strict-aliasing -funsigned-char -pipe -pthread -fno-omit-frame-pointer -fstack-protector --param=ssp-buffer-size=4 -Wtype-limits -march=native -O2 -pipe -Wno-deprecated -Wno-covered-switch-default -Wno-unnamed-type-template-args -Wno-c++11-narrowing -std=gnu++0x -std=gnu++0x  -c ../../dictionary/user_dictionary.cc -o obj/dictionary/user_dictionary.user_dictionary.o
In file included from ../../dictionary/user_dictionary.cc:51:
/var/tmp/portage/app-i18n/mozc-9999/work/mozc-9999/src/dictionary/user_dictionary_storage.h:77:7: error: cannot derive from ‘final’ base ‘mozc::user_dictionary::UserDictionaryStorage’ in derived type ‘mozc::UserDictionaryStorage’
 class UserDictionaryStorage : public user_dictionary::UserDictionaryStorage {
       ^~~~~~~~~~~~~~~~~~~~~
FAILED: obj/engine/engine.engine.o 
/usr/bin/x86_64-pc-linux-gnu-g++ -MMD -MF obj/engine/engine.engine.o.d -DOS_LINUX -DMOZC_BUILD -DCHANNEL_DEV -DENABLE_GTK_RENDERER -DNDEBUG -DQT_NO_DEBUG -DNO_LOGGING -DIGNORE_HELP_FLAG -DIGNORE_INVALID_FLAG -I/var/tmp/portage/app-i18n/mozc-9999/work/mozc-9999/src -Igen -Igen/proto_out -Wall -Wno-char-subscripts -Wno-sign-compare -Wno-deprecated-declarations -Wwrite-strings -Wno-unknown-warning-option -Wno-inconsistent-missing-override -fPIC -fno-exceptions -fmessage-length=0 -fno-strict-aliasing -funsigned-char -pipe -pthread -fno-omit-frame-pointer -fstack-protector --param=ssp-buffer-size=4 -Wtype-limits -march=native -O2 -pipe -Wno-deprecated -Wno-covered-switch-default -Wno-unnamed-type-template-args -Wno-c++11-narrowing -std=gnu++0x -std=gnu++0x  -c ../../engine/engine.cc -o obj/engine/engine.engine.o
In file included from ../../engine/engine.cc:59:
/var/tmp/portage/app-i18n/mozc-9999/work/mozc-9999/src/prediction/user_history_predictor.h:64:7: error: cannot derive from ‘final’ base ‘mozc::user_history_predictor::UserHistory’ in derived type ‘mozc::UserHistoryStorage’
 class UserHistoryStorage : public mozc::user_history_predictor::UserHistory {
       ^~~~~~~~~~~~~~~~~~

This is related to protocolbuffers/protobuf#5869.

@Arfrever
Copy link
Author

Protobuf upstream reverted this incompatible change in Protobuf 3.7.1 and deferred reintroduction of this incompatible change to unspecified future version of Protobuf.

Mozc should still be fixed to not inherit from generated classes.
https://developers.google.com/protocol-buffers/docs/cpptutorial#parsing-and-serialization says:

Protocol Buffers and O-O Design Protocol buffer classes are basically dumb data holders (like structs in C); they don't make good first class citizens in an object model. If you want to add richer behaviour to a generated class, the best way to do this is to wrap the generated protocol buffer class in an application-specific class. Wrapping protocol buffers is also a good idea if you don't have control over the design of the .proto file (if, say, you're reusing one from another project). In that case, you can use the wrapper class to craft an interface better suited to the unique environment of your application: hiding some data and methods, exposing convenience functions, etc. You should never add behaviour to the generated classes by inheriting from them. This will break internal mechanisms and is not good object-oriented practice anyway.

@Arfrever Arfrever changed the title Build failure with Protobuf 3.7.0 Build failure with Protobuf 3.7.0 and unspecified future version Apr 20, 2019
teto pushed a commit to NixOS/nixpkgs that referenced this issue Apr 24, 2019
protobuf was bumped to 3.7.1, which fixes the build issue as described
in google/mozc#460.

This reverts commit a19fbaf.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 19, 2020
@Arfrever
Copy link
Author

Similar fix was applied in commit a1dcada:

commit a1dcadabd3a1c604e8beff1e45830c1d9adfce84
Author:     Hiroyuki Komatsu <[email protected]>
AuthorDate: Sat Aug 22 21:29:00 2020 +0900
Commit:     Hiroyuki Komatsu <[email protected]>
CommitDate: Sat Aug 22 21:29:00 2020 +0900

    Update from the upstream repository.

--- /src/dictionary/user_dictionary_storage.h
+++ /src/dictionary/user_dictionary_storage.h
...
@@ -72,9 +72,7 @@
 class Mutex;
 class ProcessMutex;
 
-// Inherit from ProtocolBuffer
-// TODO(hidehiko): Get rid of this implementation.
-class UserDictionaryStorage : public user_dictionary::UserDictionaryStorage {
+class UserDictionaryStorage {
  public:
   typedef user_dictionary::UserDictionary UserDictionary;
   typedef user_dictionary::UserDictionary::Entry UserDictionaryEntry;
...
@@ -179,17 +172,30 @@
   // maximum number of entries one dictionary can hold
   static size_t max_entry_size();
 
-  static string default_sync_dictionary_name();
+  static std::string default_sync_dictionary_name();
+
+  user_dictionary::UserDictionaryStorage &GetProto() { return proto_; }
+
+  const user_dictionary::UserDictionaryStorage &GetProto() const {
+    return proto_;
+  }
+
+  size_t dictionaries_size() const { return proto_.dictionaries_size(); }
+
+  const user_dictionary::UserDictionary &dictionaries(size_t i) const {
+    return proto_.dictionaries(i);
+  }
 
  private:
   // Return true if this object can accept the given dictionary name.
   // This changes the internal state.
-  bool IsValidDictionaryName(const string &name);
+  bool IsValidDictionaryName(const std::string &name);
 
   // Load the data from file_name actually.
   bool LoadInternal();
 
-  string file_name_;
+  user_dictionary::UserDictionaryStorage proto_;
+  std::string file_name_;
   bool locked_;
   UserDictionaryStorageErrorType last_error_type_;
   std::unique_ptr<Mutex> local_mutex_;
...
--- /src/prediction/user_history_predictor.h
+++ /src/prediction/user_history_predictor.h
...
@@ -61,19 +62,33 @@
 class UserHistoryPredictorSyncer;
 
 // Added serialization method for UserHistory.
-class UserHistoryStorage : public mozc::user_history_predictor::UserHistory {
+class UserHistoryStorage {
  public:
-  explicit UserHistoryStorage(const string &filename);
+  explicit UserHistoryStorage(const std::string &filename);
   ~UserHistoryStorage();
 
   // Loads from encrypted file.
   bool Load();
 
   // Saves history into encrypted file.
-  bool Save() const;
+  bool Save();
+
+  // Deletes entries before the given timestamp.  Returns the number of deleted
+  // entries.
+  int DeleteEntriesBefore(uint64 timestamp);
+
+  // Deletes entries that are not accessed for 62 days.  Returns the number of
+  // deleted entries.
+  int DeleteEntriesUntouchedFor62Days();
+
+  mozc::user_history_predictor::UserHistory &GetProto() { return proto_; }
+  const mozc::user_history_predictor::UserHistory &GetProto() const {
+    return proto_;
+  }
 
  private:
   std::unique_ptr<storage::StringStorageInterface> storage_;
+  mozc::user_history_predictor::UserHistory proto_;
 };
 
 // UserHistoryPredictor is NOT thread safe.
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant