Skip to content
This repository was archived by the owner on Dec 18, 2022. It is now read-only.

[Build] Allow building without Conan#234

Merged
n0toose merged 1 commit intotenacityteam:masterfrom
AnotherFoxGuy:optional_conan
Jul 30, 2021
Merged

[Build] Allow building without Conan#234
n0toose merged 1 commit intotenacityteam:masterfrom
AnotherFoxGuy:optional_conan

Conversation

@AnotherFoxGuy
Copy link
Contributor

@AnotherFoxGuy AnotherFoxGuy commented Jul 12, 2021

fixes: #75

PR for building without having Conan installed

STATUS:
Builds and runs on Manjaro with wxgtk3-dev 3.1.5-2 (See: https://github.com/tenacityteam/tenacity/pull/234#issuecomment-884876066)

Contributor stuff

✅ The code in this pull request is licensed under "GPLv2 or any later version"*
✅ I made sure the code compiles on my machine
✅ I made sure there are no unnecessary changes in the code*
✅ I made sure the title of the PR reflects the core meaning of the issue you are solving*
✅ I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"*

@Be-ing
Copy link
Contributor

Be-ing commented Jul 12, 2021

Thanks for working on this but I am in doubt this is a good direction to continue. Audacity is planning more changes to their dependency management, so I think it may be difficult to maintain this in the future. Judging from what they have done so far and their continued snide remarks towards Linux users I have little confidence those changes will turn out very well. I think it will be easier to maintain to bypass the Conan mess entirely as #228 does.

@nbsp
Copy link

nbsp commented Jul 16, 2021

What is the status of this PR? #228 has made significant progress on the same front. If this is no longer actively developed, please close this PR.

@n0toose
Copy link
Member

n0toose commented Jul 16, 2021

To be honest, it could be good if the USE_CONAN variable was still there, so some of his (much appreciated, BTW) efforts can still be used, carried on and adjusted to the more recent/newer changes.

@AnotherFoxGuy
Copy link
Contributor Author

What is the status of this PR?

It builds with wxWidgets V3.1.5, but I haven't tested if it runs on linux

@AnotherFoxGuy AnotherFoxGuy marked this pull request as ready for review July 16, 2021 18:29
@n0toose
Copy link
Member

n0toose commented Jul 22, 2021

This may be very important for our first release. See: https://github.com/tenacityteam/tenacity/pull/300#issuecomment-884782522

@AnotherFoxGuy
Copy link
Contributor Author

Just tested this on manjaro with wxgtk3-dev 3.1.5-2, seems to work fine
Screenshot_20210722_142952

n0toose
n0toose previously approved these changes Jul 22, 2021
@nbsp
Copy link

nbsp commented Jul 23, 2021

Could this be rebased into one commit Allow building without Conan? I doubt the bug fix warrants its own commit

@n0toose
Copy link
Member

n0toose commented Jul 26, 2021

FYI: 🎉 emojis look good on the GitHub front-end but weird on the Git CLI and other frontends.

I'd suggest changing the :tada: to an actual emoji or getting rid of it altogether?

@n0toose
Copy link
Member

n0toose commented Jul 26, 2021

Building this without Conan seems to work.

@AnotherFoxGuy AnotherFoxGuy changed the title 🎉 Build without conan 🔨 Allow building without Conan Jul 26, 2021
@AnotherFoxGuy AnotherFoxGuy changed the title 🔨 Allow building without Conan [Build] Allow building without Conan Jul 26, 2021
n0toose
n0toose previously approved these changes Jul 26, 2021
Copy link
Member

@n0toose n0toose left a comment

Choose a reason for hiding this comment

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

Seems good to me, I think building this with Conan using the correct parameter needs some further testing.

Be-ing
Be-ing previously requested changes Jul 26, 2021
Copy link
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

Please rename Findlibmp3lame.cmake to Findmp3lame.cmake for consistency with the CMake target name in vcpkg.

@AnotherFoxGuy
Copy link
Contributor Author

AnotherFoxGuy commented Jul 28, 2021

@Be-ing Please rename Findlibmp3lame.cmake to Findmp3lame.cmake for consistency with the CMake target name in vcpkg.

Renaming lame will completely break the buildsystem, I will open a separate PR to fix it
(Also, vcpkg is the only one that calls it mp3lame)

@AnotherFoxGuy AnotherFoxGuy requested a review from Be-ing July 28, 2021 09:31
@fossdd fossdd self-requested a review July 28, 2021 13:01
Signed-off-by: Edgar <Edgar@AnotherFoxGuy.com>
Copy link
Member

@fossdd fossdd left a comment

Choose a reason for hiding this comment

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

Builds and works.

Copy link
Member

@n0toose n0toose left a comment

Choose a reason for hiding this comment

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

Edgar, this seriously rocks.

@n0toose n0toose dismissed Be-ing’s stale review July 30, 2021 23:20

This has been resolved.

@n0toose n0toose merged commit 04609bb into tenacityteam:master Jul 30, 2021
@AnotherFoxGuy AnotherFoxGuy deleted the optional_conan branch July 31, 2021 05:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Conan optional

5 participants