roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

Implement backtrace printing for non-glibc targets #265

Closed hrishikeshSuresh closed 5 years ago

hrishikeshSuresh commented 5 years ago

249

Still working on this, just referenced this PR.

gavv commented 5 years ago

/link #242

hrishikeshSuresh commented 5 years ago

The -ldl & -lunwind flags must be passed while compiling the backtrace.cpp codes for target_libunwind & target_bionic respectively. Should these go as args for env.ParsePkgConfig() in SConstruct?

hrishikeshSuresh commented 5 years ago

The -ldl & -lunwind flags must be passed while compiling the backtrace.cpp codes for target_libunwind & target_bionic respectively. Should these go as args for env.ParsePkgConfig() in SConstruct?

Okay, just saw your comment. I understood as to how to solve this.

gavv commented 5 years ago

I've tested the Android version. It didn't work out of the box, but worked after fixing compiler options a bit: f6e049cf432b67ae717b7636364c36c52329cf14. This is pushed to develop.

hrishikeshSuresh commented 5 years ago

I found a few errors (broken link, missing flags etc.) while downloading dependencies. These could be platform specific. Should I fix them in a separate PR?

gavv commented 5 years ago

I found a few errors (broken link, missing flags etc.) while downloading dependencies. These could be platform specific. Should I fix them in a separate PR?

Yep, would be great.

gavv commented 5 years ago

Looks good!

But the addition of -pthread and -lm to sndfile and cpputest looks suspicious. These libraries should be able to build themselves. Probably it's something wrong with our build script or maybe with your environment. Probably we need to fix it some other way. I suggest to remove these two pieces from the PR and open a separate issue or PR. It would help if you will try to reproduce the problem outside of roc. If it is reproducing outside, this fix is okay. Otherwise, we should investigate why.

Also please rebase your branch on fresh develop branch and squash commits into one or two (one for libunwind + scons and one for bionic), and then we can merge this.

gavv commented 5 years ago

It seems you forgot to pull a fresh develop branch from upstream before rebasing on it. It has 5 new commits and there are conflicts.

hrishikeshSuresh commented 5 years ago

Yeah I'm trying to re-base it properly & fix the conflicts. Will be done.

gavv commented 5 years ago

OK, ping me when it will be ready for merge.

hrishikeshSuresh commented 5 years ago

I pulled the fresh develop branch but I can't squash the commits properly because I was earlier using another branch backtrace_print and if I checkout to an old commit and try to squash commits, I can't merge the detached head with develop because of merge conflicts. Is it very important to squash the commits? I tried doing this via git kraken.

gavv commented 5 years ago

Yes, it's quite important for us because our goal is to keep the git history linear and human-readable.

Here is how you can do it in your case:

# get a fresh copy of your fork
$ git clone https://github.com/hrishikeshSuresh/roc.git
$ cd roc

# checkout develop branch from your fork
$ git checkout develop

# add upstream repo (call it "upstream") and fetch its branches
$ git remote add upstream https://github.com/roc-project/roc.git
$ git fetch upstream

Here is your history now (you can see it using "gitk" command):

image

Then:

# find best common ancestor between upstream/develop and your develop
$ git merge-base upstream/develop develop
c7cb95937947a8a02ebe10c7f752d1cd979a2c47

# rebase your develop on that commit
$ git rebase c7cb95937947a8a02ebe10c7f752d1cd979a2c47

Here is your history after rebase. As you see, it became linear:

image

Then:

# choose your text editor here
$ export GIT_EDITOR=vim

# run an interactive rebase
# we want to rebase develop branch from your fork on the develop branch from upstream
$ git rebase -i upstream/develop

The last command will spawn an editor where you can edit the history. You'll need to change "pick" to "squash" in every line except the very first one:

image

And then save your changes and exit the editor. After that the editor will be spawned once more and you'll have to edit the commit message, like this:

image

Then, again, you save your changes and exit the editor.

After that the rebase finishes. Here is your history now:

image

As you can see, it is a single commit on top of upstream/develop.

Now you can push it to your fork:

git push -f origin develop
gavv commented 5 years ago

BTW, personally I'd recommend to avoid using GUI tools to manipulate git history. In my experience, they usually only give an illusion of simplicity and can't really exempt from understanding how git works in any non-trivial case.

gavv commented 5 years ago

Also, don't forget to remove -pthread and -lm from 3rdparty.py (for sndfile and cpputest), as I wrote above. You can do it after rebase and then use git commit --ammend.

hrishikeshSuresh commented 5 years ago

It worked now. Earlier, I did not make the history linear and I was just squashing all commits. Now I understand how to work with such issues. Thank you!

gavv commented 5 years ago

You're welcome. LGTM. Can we merge this?

hrishikeshSuresh commented 5 years ago

Yes. Everything is done.

gavv commented 5 years ago

Thanks for finishing this!

gavv commented 5 years ago

Follow-up commit (formatting): 856ecf6eacbfb748eef4015d1c732950e4ea58e2