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

ABI and SONAME need a bump #432

Closed
agaida opened this issue Jun 1, 2019 · 11 comments · Fixed by #536
Closed

ABI and SONAME need a bump #432

agaida opened this issue Jun 1, 2019 · 11 comments · Fixed by #536
Assignees

Comments

@agaida
Copy link
Member

agaida commented Jun 1, 2019

Current Behavior

We introduced a backward incompatible change with exif and jpg handling

@@ -289,6 +289,7 @@ libfm-qt.so.6 libfm-qt6 #MINVER#
  (c++)"Fm::FileDialog::setViewMode(Fm::FolderView::ViewMode)@Base" 0.12.0
  (c++)"Fm::FileDialog::splitterPos() const@Base" 0.12.0
  (c++)"Fm::FileDialog::staticMetaObject@Base" 0.12.0
+ (c++)"Fm::FileDialog::suffix(bool) const@Base" 0.14.2~
  (c++)"Fm::FileDialog::updateAcceptButtonState()@Base" 0.12.0
  (c++)"Fm::FileDialog::updateSaveButtonText(bool)@Base" 0.12.0
  (c++)"Fm::FileDialog::updateSelectionMode()@Base" 0.12.0
@@ -678,6 +679,7 @@ libfm-qt.so.6 libfm-qt6 #MINVER#
  (c++)"Fm::FolderModelItem::FolderModelItem(Fm::FolderModelItem const&)@Base" 0.10.0
  (c++)"Fm::FolderModelItem::FolderModelItem(std::shared_ptr<Fm::FileInfo const> const&)@Base" 0.12.0
  (c++)"Fm::FolderModelItem::bindCutFiles(std::shared_ptr<std::set<unsigned int, std::less<unsigned int>, std::allocator<unsigned int> > const> const&)@Base" 0.12.0
+ (c++)"Fm::FolderModelItem::displayDtime() const@Base" 0.14.2~
  (c++)"Fm::FolderModelItem::displayMtime() const@Base" 0.12.0
  (c++)"Fm::FolderModelItem::displaySize() const@Base" 0.12.0
  (c++)"Fm::FolderModelItem::findThumbnail(int, bool)@Base" 0.12.0
@@ -1034,7 +1036,7 @@ libfm-qt.so.6 libfm-qt6 #MINVER#
  (c++)"Fm::ThumbnailJob::qt_metacast(char const*)@Base" 0.12.0
  (c++|arch= !armel !armhf !i386 !mips !mipsel !hppa !hurd-i386 !kfreebsd-i386 !m68k !powerpc !powerpcspe !sh4 !x32 )"Fm::ThumbnailJob::readImageFromStream(_GInputStream*, unsigned long)@Base" 0.12.0
  (c++|arch= !amd64 !arm64 !mips64el !ppc64el !s390x !alpha !ia64 !kfreebsd-amd64 !ppc64 !riscv64 !sparc64 )"Fm::ThumbnailJob::readImageFromStream(_GInputStream*, unsigned int)@Base" 0.12.0
- (c++)"Fm::ThumbnailJob::readJpegExif(_GInputStream*, QImage&, int&)@Base" 0.12.0
+ (c++)"Fm::ThumbnailJob::readJpegExif(_GInputStream*, QImage&, QMatrix&)@Base" 0.14.2~
  (c++)"Fm::ThumbnailJob::setLocalFilesOnly(bool)@Base" 0.14.0~
  (c++)"Fm::ThumbnailJob::setMaxThumbnailFileSize(int)@Base" 0.14.0~
  (c++)"Fm::ThumbnailJob::staticMetaObject@Base" 0.12.0
Possible Solution

bump both

Context

pcmanfm-qt will try to use the removed symbol and fail

System Information

latest git

@tsujan
Copy link
Member

tsujan commented Jun 1, 2019

See #428:

NOTE: pcmanfm-qt should be recompiled, preferably...

Please also check my pcmanfm-qt's PRs and if good, merge them (I use them for a while).

@tsujan
Copy link
Member

tsujan commented Jun 1, 2019

libfm-qt's patches often require recompilation of all libfm-qt based apps.

@agaida
Copy link
Member Author

agaida commented Jun 1, 2019

here the case is different - let's have a look at the diff

- (c++)"Fm::ThumbnailJob::readJpegExif(_GInputStream*, QImage&, int&)@Base" 0.12.0
+ (c++)"Fm::ThumbnailJob::readJpegExif(_GInputStream*, QImage&, QMatrix&)@Base" 0.14.2~

for additional symbols i can just increment the minor number, but this redefined symbol break backward compatibility - so a bump to a new major is needed - only one time as we don't care if we break compat within development cycle.

@yan12125
Copy link
Member

yan12125 commented Jun 1, 2019

For this specific case, I think the whole ThumbnailJob class should be hidden. Of course this is also an ABI-breaking change.

@tsujan
Copy link
Member

tsujan commented Jun 1, 2019

Fm::ThumbnailJob::readJpegExif() is a private method. In this sense, that doesn't break backward compatibility (it's sometimes called "source compatible"). In the case of other senses, you're the master.

That being said, sometimes it's necessary to break backward compatibility in the code (not for users), in which case, I explicitly tell about it in my PRs.

@tsujan
Copy link
Member

tsujan commented Jun 1, 2019

In other words, the code should be enhanced without caring about developers (like Gnome devs do) but with the utmost care for users (unlike what Gnome devs do). That's what I've always done — not my first time.

Versioning is outside my realm though.

@agaida
Copy link
Member Author

agaida commented Jun 1, 2019

the bump will be needed somewhen just to prevent situations in which someone try a new libfm-qt with and old pcmanfm-qt - the cleanest way is to bump the soname version before a release (aka now) - if one compile things hisself nothing bad happends, but the very most distribution maintainers will see big fat warnings about this change and can react to it.

EDIT: So - the mentioned change is nothing bad, we should just not care about 😄 - but we shouldn't forget to bump before a release.

@agaida
Copy link
Member Author

agaida commented Jun 1, 2019

Hmm - if the method is private and is not used outside of libfm-qt it shouldn't be exported first hand if possible.

@tsujan
Copy link
Member

tsujan commented Jun 1, 2019

but we shouldn't forget to bump before a release.

I've changed ABI and API a lot but have never interfered with LXQt versioning process.

If I become responsible for versioning libfm-qt/pcmanfm-qt, I'll do it as in my projects (simultaneously releasing them every 2 months or so and bumping their versions after each release — and sometimes bumping it again, depending on the work I've done or my mood). So, never trust me when it comes to versioning ;) You're the boss there.

@agaida agaida self-assigned this Jun 2, 2019
@agaida
Copy link
Member Author

agaida commented Jun 3, 2019

Unrelated to our issue, but sometimes i think the same (bits from #d-qt-kde):

2019-06-02  20:46:38 <lisandro> that's exactly why I always say one should do a   
                     "maintainer boot camp" for developers
2019-06-02  20:47:25 <svuorela> heh

@tsujan
Copy link
Member

tsujan commented Jun 3, 2019

:)

agaida added a commit that referenced this issue Jun 8, 2019
tsujan added a commit that referenced this issue Apr 21, 2020
tsujan added a commit that referenced this issue Apr 23, 2020
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

Successfully merging a pull request may close this issue.

3 participants