Re: wip/soundfont (and wip/ branches)

[Tim Janik <timj gnu org>]

On 21.09.2016 21:08, Stefan Westerfeld wrote:
> > If you want me to guess, it could be related to bse_object_new now hardcoding the C++ object types that 
> > are paired with a GType object.
> > But you're right it needs further investigation.
>
> Fixed, by replacing g_object_new with bse_object_new, see below.

Ok.

> I tried it, while merging master into wip/soundfont-merge there are the following
> problems:
>
>  * git diff [ foo.cc ] will show all changes in master, that is a lot
>  * it can be workarounded using explicit git diff master [ foo.cc ]
>  * also, git status produces a looong list of stuff, due to the number
>    of changes in master
>
> also after the merge gitk will show every change in master if you select
> the merge commit and look at the history.
>
> So the next thing I tried was reversing it, and create wip/soundfont-merge
> as proposed:
>
>  * git diff shows the expected thing (diffs created by soundfont code)
>  * git status shows the expected thing (status changes by soundfont code)
>
> after merge gitk will show the soundfont branch history.

Hm, gitk shows a combined diff for the merge... Just learning how to read that from git-diff(1) ;-)

With git diff, I suspect you view this:
  git diff 1698dc960d2607323551a72c59d581efdbe6e1fc^!
I.e.:
  *   1698d # Merge branch 'wip/soundfont' into wip/soundfont-merge (8 days ago) <Stefan Westerfeld>
Right?

> So it seems that to git, the merging process is not completely symmetric and
> its better to think of it as merging a topic-like branch onto a master-like
> branch.

Yeah, I think that's simply dependent on the order of the merge parents in the merge commit.
What I meant was the file tree should end up the same, right?

> Anyway, you can now pull the branch (produced in this way)
>
>     wip/soundfont-merge

Merge commit:
>   Merge branch 'wip/soundfont' into wip/soundfont-merge
>
>    This merge creates a version of the wip/soundfont branch that cleanly applies
>    to the current master branch.

This should say something like "merge with 'master'" (like the description
mentions), instead of referring to a temporary branch that'll never be pushed
into the public beast repo.

> --------------------------- bse/bseproject.genprc.cc ---------------------------

This should have never been checked in.

> +++ bse/bseproject.proc
> + METHOD (BseProject, get-sound-font-repo) {

Please try to move this into bseapi.idl, we're trying to get rid of all *.proc files.

> diff --cc bse/bsesoundfontrepo.proc

Same here, please try adding this to bseapi.idl.

> it also contains two additional commits after the merge:
>
>  * the g_object_new triggering assertion problem
>  * I also moved the replay network from zintern/ to res/.

This looks wierd, master doesn't have zintern/Makefile.am anymore, your merge
commit doesn't seem to reintroduce it, but the

> so I was able to test that loading soundfonts and playback works.

Ok, that's an important step already, thanks so far.

The merge doesn't apply cleanly to my local tree which has a bunch more proc->bseapi.idl moves in it.

I'll do my best to push this to github in the net few days and can need your help resolving the conflicts 
after that.

>    Cu... Stefan

-- 
Yours sincerely,
Tim Janik

https://testbit.eu/timj/
Free software author.