[Gc] Should libatomic_ops be inside bdwgc?
urkedal at nbi.dk
Fri Aug 12 09:42:29 PDT 2011
On 2011-08-12, Ivan Maidanski wrote:
> Hi Petter,
> Now I have another issue with make distcheck (I have libatomic_ops folder inside bdwgc):
> configure: error: libatomic_ops is required. You can either install it on your system, or fetch and unpack a resent version into the source directory and link or rename it to libatomic_ops.
The issue is that distcheck needs everything to be automated, so I think
the option of unpacking libatomic_ops in the source directory has to be
secondary. We'll need to use a pre-installed one ourselves. To test
that the other option worked, I passed --without-libatomic-ops and used
the regular build target.
> 12 08 2011, 00:17 Petter Urkedal <urkedal at nbi.dk>:
> > On 2011-08-11, Ivan Maidanski wrote:
> > > Thanks. Just merged. (I think you've already received a notification).
> > Seems that's not one of the notifications they support. Good you merged
> > it before anything else, so I could fast-forward; I made the mistake of
> > putting it in my master branch.
> IMHO, This is not a mistake - it's a good practice to send pull requests from your master branch (which you supposed to have tested already).
I disagree. The issue is that I want to keep my master in sync with
yours, so that we have an official history. If you had a committed
something before the merge, that would mean I'd have to rebase and
force-update to keep in sync. That's a very bad thing if someone was
using my master branch. So, my guess is that the recommended practise
is to create topic (throw-away) branches for pull requests.
That does not mean it can't be properly tested by the requester. Often
one uses a "next" branch, which integrates topic-branches for testing
before they are (selectively) merged into "master". "next" can get
messy. To keep "master" more linear one can ask contributors to rebase
their changes before merging them into "master".
> It's really unclear to me how to handle pull requests in github - I'd to make check before you do merge but now I do merge then pull and make check. (i.e. I can't switch to your branch w/o cloning your repo).
You can test my branch in advance by adding
git remote add paurkedal git://github.com/paurkedal/bdwgc.git
git fetch paurkedal [BRANCH]
Now you can checkout and tests, but of course, don't base any work on my
topic-branches, as I may force-update them. The fetch is fairly
light-weight, since our repos are almost the same.
I'm new to the collaborative github features myself. The documentation
for Git is very precise (esp I like the Pro Git book), but it would be
good to have some more best-practice advice for collaboration on github
or elsewhere when topic branches have to be public. E.g. I figured out
from a discussion that it was okay to force-update a pull request.
> And, after merge, we have a triangle in git commit history - look at it with gitk --all.
> Any advice how to do it (merge pull request) sequentially? (This is not a problem of course but...)
I've disabled Tk in my build, but I think "git log --graph" shows what
you mean. I was a bit puzzled about that extra commit added when
merging a pull request. If the merge is non-trivial, then it's needed,
as it contains any automatic changes from the merging algorithm and
manual changes after conflicts. However, in this case we had a
fast-forward merge, so the only thing contained in the last commit was
the log message. Without that, the triangle could be collapsed. Maybe
the way it works is for the best, since it allows recording both the
author and the committer, and treats real merges the same as
fast-forward merges. Also, rebasing to keep the history linear make be
unpractical when there are many contributors.
> > > > > 1. Based on the answer from Hans, could you prepare the relevant patch for the scripts (including any other things you think need adjusting, if any)? Thanks.
> > There wasn't much to it, just making sure the libatomic_ops directory is
> > there if we need to use it.
> Another Q: what do think of:
> ## FIXME: really needed? (AC_LIBTOOL already provides this)
> AC_CHECK_TOOL(AR, ar)
> AC_CHECK_TOOL(RANLIB, ranlib, :) # :)
Yes, those can be removed. I'll have a closer look later this weekend.
More information about the Gc