Skip to content
This repository has been archived by the owner on May 12, 2018. It is now read-only.

Revert "Merge pull request #386 ... gpb-recompilation...detection" #413

Merged
merged 1 commit into from
Dec 30, 2014

Conversation

tomas-abrahamsson
Copy link
Contributor

As per discussions with Tuncer Ayaz and Luis Rascão, partly in #410,
and partly as an off-github spinoff private discussion, this is a request to
revert the merge of #386, since it changed the gpb compiler to
not use rebar_base_compiler. That change was to add support for
target name prefixes, but it is better to add that support to
rebar_base_compiler.

…pilation-detection"

This reverts commit 81063d3, reversing
changes made to 6584def.

This was reverted because it changed the gpb compiler to
not use rebar_base_compiler.  That change was to add support for
target name prefixes, but it is better to add that support to
rebar_base_compiler.
@ferd
Copy link
Contributor

ferd commented Dec 23, 2014

Well this breaks the build.

@ghost
Copy link

ghost commented Dec 23, 2014

The build error is only for R16B and I'm not sure about the cause. Could it be a timeout in inttest/port_rt, and can we trigger a rebuild to see if that's it?

@tomas-abrahamsson
Copy link
Contributor Author

It does indeed break the build. It fails for R16B only, in the port_rt inttest, testing compilation of C code. It seems to fail since the rebar compile process exits with exit code 130, which is 128+INT, after about 40 seconds of inttest execution (all inttests).

There are other successful cases where the inttest has proceeded longer. It is unclear, due to lack of timestamps, how long this particular port_rt inttest has been running, but other timestamps in earlier inttests indicate it cannot have been more than 25 seconds, so it should have been safe from inttest's default timeout of 30s.

I don't understand why, and I don't see how it could have happened as part of changing the recompilation strategy for protobuf files. Agree with @Tuncer about trying a rebuild. Is that anything I should do to trigger it?

@ghost
Copy link

ghost commented Dec 23, 2014

Not sure if there's a way to do it on travis-ci, but pushing a modified branch will definitely trigger a build. Maybe you can add a temporary debug log in that test and use that as the branch modification.

@tomas-abrahamsson
Copy link
Contributor Author

I added a debug-printout to port_rt.erl, and travis passed the tests. https://travis-ci.org/rebar/rebar/builds/44984703. So I did a reset to the previous commit, hoping this would trigger a travis-build with the branch in the shape I would want it to have, but github apparently remembered the previous travis build as well, hence it is still marked as failed. Oh so clever github :)

@tomas-abrahamsson
Copy link
Contributor Author

@ghost
Copy link

ghost commented Dec 23, 2014

Try again, GitHub was unavailable.

@ferd
Copy link
Contributor

ferd commented Dec 24, 2014

Yeah that appears to work. I can defer to you and merge this whenever.

@ghost
Copy link

ghost commented Dec 24, 2014

Looks good to me.

@ghost
Copy link

ghost commented Dec 30, 2014

Can we land this in master? It's a prerequisite for the proper fix.

ferd added a commit that referenced this pull request Dec 30, 2014
Revert "Merge pull request #386 ... gpb-recompilation...detection"
@ferd ferd merged commit b8e0018 into rebar:master Dec 30, 2014
@ferd
Copy link
Contributor

ferd commented Dec 30, 2014

This broke the build in master, but so did reversing the merge (in its own PR, see https://travis-ci.org/rebar/rebar/builds/45475087). Not sure what's to blame. I'm gonna try again later and see what happens.

@ferd
Copy link
Contributor

ferd commented Dec 30, 2014

Tests have appeared to run fine locally at least.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants