Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Don't touch libuv's configure in it's build rule
It's the prerequisite for this rule, so touching it will invalidate the libuv for all other trees.
- Loading branch information
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vtjnash I think you added this. I haven't seen any problems after removing this. What was the problem you were trying to avoid?
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, per the comment in the file, it was added to help prevent autoconf from getting called.
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but under what circumstances was autoconf being called. I haven't seen it get called yet, ever. Do we need to change the prerequisites for this target to avoid the invalidation cycle?
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
touch -c$(SRCDIR)/srccache/$ (LIBUV_SRC_DIR)/aclocal.m4
is expected to cause it
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this broke the Fedora nightlies, since autoconf isn't installed on the build VMs:
https://copr-be.cloud.fedoraproject.org/results/nalimilan/julia-nightlies/fedora-23-x86_64/00142033-julia/build.log.gz
It would be cool not to require it, since involving autotools and m4 macros in the build is yet another potential source of failures.
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make the prerequisite for this build rule something other than configure?
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
Makefile.in?40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for example. Just wanted to confirm with @vtjnash who has looked into this before.
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
straight revert, or which file should this touch?
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worst case, a configure invalidation of touching too many times just means other trees sharing the same source checkout re-run configure which doesn't take that long for libuv. We could potentially move all 3 of these touches to immediately after the checkout/untarring of the source, though that's maybe a little messy to put in to the git-externals define.
40386c9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't take that long a time, but it's still very annoying. If you're switching back and forth between two tree, it can easily become the dominating factor.