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

Keep up with Audacity's library separation efforts (part 1) #656

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

n0toose
Copy link
Member

@n0toose n0toose commented Oct 4, 2021

Initial work with cherry-picking for the sake of keeping up with Audacity's efforts of separating libraries and maintaining compatibility.

If you wish to review this pull request, please check if I've accidentally added a part of their project that we do not want (e.g. Sentry reports) or if I've accidentally removed something ours.

Checklist
  • I have signed off my commits using -s or Signed-off-by* (See: Contributing § DCO)
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code* (please double-check)
  • 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?"*

* indicates required

@Be-ing
Copy link
Contributor

Be-ing commented Oct 4, 2021

Thank you for working on this but unilaterally declaring you will merge in a very short timespan without review is inappropriate. This is a large change, +1,830 −341, that warrants more than such a rushed process.

@n0toose
Copy link
Member Author

n0toose commented Oct 4, 2021

Thank you for working on this but unilaterally declaring you will merge in a very short timespan without review is inappropriate. This is a large change, +1,830 −341, that warrants more than such a rushed process.

Got it, will remove that part.

@Semisol Semisol added the major change This PR is a major change and requires more review label Oct 4, 2021
@Be-ing
Copy link
Contributor

Be-ing commented Oct 4, 2021

It looks like there are a few commits in the beginning which are already in the master branch? Please do an interactive rebase to remove those.

@n0toose
Copy link
Member Author

n0toose commented Oct 4, 2021

I was afraid that people weren't checking this, so it was like, "okay, I might as well spearhead this on my own for a bit". There's a small mistake where I accidentally signed some changes that weren't mine, fixing this right now.

@Semisol
Copy link
Contributor

Semisol commented Oct 4, 2021

I have added the major change label. I don't have the capacity and/or knowledge to review this so I'll leave it up to others.

@n0toose
Copy link
Member Author

n0toose commented Oct 4, 2021

Most of these changes were pulled as-is from the Audacity tree. I stopped at the part where I had to also split lib-basic-ui further (which is when the further library separations took place) because it'd make everything much more complex.

@n0toose
Copy link
Member Author

n0toose commented Oct 4, 2021

Seriously wondering why the Windows builds specifically are failing. Doesn't seem like a CI issue.

src/ProjectWindows.cpp Outdated Show resolved Hide resolved
@n0toose
Copy link
Member Author

n0toose commented Oct 4, 2021

Force-pushed. I took advantage of this opportunity to replace a single documented reference of AUDACITY_DLL_API, improve some commit messages with better pointers to original commits and notes that the patches that I specifically adapted for Tenacity were modified for Tenacity. Should be better than before.

@n0toose n0toose changed the title Merge panos/library-separation (part 1) Keep up with Audacity's library separation efforts (part 1) Oct 4, 2021
CMakeLists.txt Outdated Show resolved Hide resolved
n0toose and others added 11 commits October 5, 2021 08:04
Based off the work of Paul and adapted for Tenacity.

Original commit: 54b5f7d

Co-authored-by: Paul Licameli <paul.licameli@audacityteam.org>
Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Original commit: ab30511

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
... as suggested by Sampo Hippeläinen.

Original commit: 3ebebbb

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
We don't need many of these imported strings, because we don't have
crash reports or updates.

Commit imported from d20cf01

Co-authored-by: Paul Licameli <paul.licameli@audacityteam.org>
Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Original commit: 060885c

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Useful for eliminating many direct mentions of wxString. We added this patch
from Audacity because it coincides with our goal of becoming more platform
agnostic.

Original commits:
- cfbdd2d
- 132f04d

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
... but none of the methods is defined yet.

The intention is to inject dependencies on wxWidgets (or other) toolkit so that
lower-level files have less build dependency on wxCore classes or on the
event loop.

Original commit: d20cf01

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
...so direct uses of wx/app.h may later be removed.  That header is one of the
files that is in wxBase, but is not acceptable in "toolkit neutral" code,
which should not directly use any version of an event loop.

Original commit: 50e8139

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
... An elaborate way to hold a wxWindow* as parent of a window, but avoiding
mention of wxWindow* in the member functions of BasicUI::Services, to be defined
next

Original commit: 2facbe0

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Patch imported by Audacity and adapted for Tenacity.

Original commit: 6477c9e0a6cb93b781c6c83153f1d036a7cc4c13

Signed-off-by: Panagiotis Vasilopoulos <hello@alwayslivid.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
major change This PR is a major change and requires more review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants