Skip to content

Drop support of libtorrent versions less than 1.2.11#13783

Closed
glassez wants to merge 1 commit into
qbittorrent:masterfrom
glassez:drop-old-libtorrent
Closed

Drop support of libtorrent versions less than 1.2.11#13783
glassez wants to merge 1 commit into
qbittorrent:masterfrom
glassez:drop-old-libtorrent

Conversation

@glassez
Copy link
Copy Markdown
Member

@glassez glassez commented Nov 20, 2020

...according to what we decided earlier.

@glassez glassez added Code cleanup Clean up the code while preserving the same outcome Build system Issue with the build configuration or scripts (but not the code itself) labels Nov 20, 2020
@glassez glassez added this to the 4.3.1 milestone Nov 20, 2020
@glassez glassez requested a review from Chocobo1 November 20, 2020 18:21
@Chocobo1
Copy link
Copy Markdown
Member

@glassez glassez force-pushed the drop-old-libtorrent branch from e2cf057 to 98648d8 Compare November 21, 2020 03:33
Comment thread .github/workflows/ci.yaml Outdated
@Chocobo1
Copy link
Copy Markdown
Member

@sledgehammer999
Can you update the libtorrent library used for appveyor CI?

@glassez glassez force-pushed the drop-old-libtorrent branch 2 times, most recently from f31de83 to a5e71f9 Compare November 21, 2020 07:25
Chocobo1
Chocobo1 previously approved these changes Nov 21, 2020
Chocobo1
Chocobo1 previously approved these changes Nov 23, 2020
@FranciscoPombal
Copy link
Copy Markdown
Member

@Chocobo1 @glassez

microsoft/vcpkg#14755.

If we are serious about having vcpkg in our build toolset, we should have more influence on libtorrent port maintenance.

Look at the port history, I am responsible for basically all recent updates. I haven't invested time into a v2 port since I was waiting for proper versioning support to land, as that would enable us to sidestep problems such as figuring out whether it is best to add v2 as a separate port.

@sledgehammer999
Copy link
Copy Markdown
Member

I have updated the AppVeyor CI libs. I also restarted the PR build on the CI.

@glassez
Copy link
Copy Markdown
Member Author

glassez commented Nov 26, 2020

Someone understand what's wrong with Travis MacOS builds?

@c0re100
Copy link
Copy Markdown

c0re100 commented Nov 26, 2020

Someone understand what's wrong with Travis MacOS builds?

vcpkg still using libtorrent 1.2.10....

@Chocobo1
Copy link
Copy Markdown
Member

Chocobo1 commented Nov 27, 2020

vcpkg still using libtorrent 1.2.10....

microsoft/vcpkg#14755

Someone understand what's wrong with Travis MacOS builds?

I'll take a quick look at it soon. See PR #13848.

Comment thread src/base/bittorrent/session.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.2.12 of libtorrent will load certificates from the Windows store, thus allowing it on all platforms, so we could remove this macro and just change the check to 1.2.12. See arvidn/libtorrent#5313. Alternatively, that could be extracted to a seperate commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a separate task. It will need to be tested first before we add support for it.
In addition, if this feature is important, we will simply raise the minimum supported version of libtorrent (once libtorrent 1.2.12 is released and tested) without any conditionals.

@FranciscoPombal
Copy link
Copy Markdown
Member

@glassez Now that the vcpkg PR is merged, I cleaned up my original PR, see #13511 (comment).

@glassez glassez force-pushed the drop-old-libtorrent branch from bf0a7aa to 5699b93 Compare December 4, 2020 04:04
@glassez
Copy link
Copy Markdown
Member Author

glassez commented Dec 4, 2020

PR is rebased on top of current master.

@glassez
Copy link
Copy Markdown
Member Author

glassez commented Dec 4, 2020

Now GHA CI fails because of something else that has nothing to do with qBittorrent (actually due to vcpkg issues). It seems that while what should help the development, only prevents us from moving forward... I became convinced that we should not be in a hurry to discard the old proven things.

@ArcticGems
Copy link
Copy Markdown

Now GHA CI fails because of something else that has nothing to do with qBittorrent (actually due to vcpkg issues). It seems that while what should help the development, only prevents us from moving forward... I became convinced that we should not be in a hurry to discard the old proven things.

FranciscoPombal used Qt: 5.15.1 in 54b1159 (you used Qt: 5.15.0), could that be the issue???

@glassez
Copy link
Copy Markdown
Member Author

glassez commented Dec 4, 2020

FranciscoPombal used Qt: 5.15.1 in 54b1159 (you used Qt: 5.15.0), could that be the issue???

I don't touch Qt version here. It is provided by vcpkg, IIRC.

@FranciscoPombal
Copy link
Copy Markdown
Member

@glassez

Now GHA CI fails because of something else that has nothing to do with qBittorrent (actually due to vcpkg issues). It seems that while what should help the development, only prevents us from moving forward... I became convinced that we should not be in a hurry to discard the old proven things.

Lol, occasional vcpkg failures are the least holding us back. If you want to move forward, take action: microsoft/vcpkg#14941. Interesting how after 500+ painless and quick CI jobs in production vcpkg + GH Actions is not "proven", but the "old proven things" that were sometimes broken for months at time are (remember the macOS Travis job?).

I don't touch Qt version here. It is provided by vcpkg, IIRC.

If you touch vcpkg, you likely touch the Qt version (this is the case here, this upgrade bumps it from 5.15.0 to 5.15.1), so the version in that comment should be bumped. That comment is there so that it is clear what important versions of things that we care about correspond to the particular vcpkg commit.

@glassez
Copy link
Copy Markdown
Member Author

glassez commented Dec 4, 2020

Interesting how after 500+ painless and quick CI jobs in production vcpkg + GH Actions is not "proven", but the "old proven things" that were sometimes broken for months at time are (remember the macOS Travis job?).

My comment I became convinced that we should not be in a hurry to discard the old proven things. referred to your general attitude towards progress by immediately discarding all the old in favor of the new.
It seems that while what should help the development, only prevents us from moving forward... applies to all of our CIs.

@FranciscoPombal
Copy link
Copy Markdown
Member

@glassez

My comment I became convinced that we should not be in a hurry to discard the old proven things. referred to your general attitude towards progress by immediately discarding all the old in favor of the new.

It's not like this is such a complete and utter breakage that it forces us to stop using vcpkg. IMO it has more than proven itself by now. After a while, maintaining the previous system in addition to the new one that works better feels like unnecessary extra work.

It seems that while what should help the development, only prevents us from moving forward... applies to all of our CIs.

I see, I stand corrected and apologize for my assumptions. But in that case, I would still characterize your opinion of CI's as overly negative. Seems to me that they are very much worth having, despite the occasional maintenance headache.

@glassez glassez closed this Dec 5, 2020
@glassez glassez deleted the drop-old-libtorrent branch December 7, 2020 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build system Issue with the build configuration or scripts (but not the code itself) Code cleanup Clean up the code while preserving the same outcome

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants