New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

master won't compile #1200

Closed
jpdna opened this Issue Oct 8, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@jpdna
Member

jpdna commented Oct 8, 2016

As of d70e68b from earlier today
ADAM master won't compile for me

[ERROR] /home/jp/test_compile/v1/adam/adam-core/src/test/scala/org/bdgenomics/adam/rdd/ADAMContextSuite.scala:297: error: not found: value resourcePath
[ERROR]     val inputPath = resourcePath("HLA_DQB1_05_01_01_02.fa")
[ERROR]                     ^
[ERROR] one error found
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] ADAM_2.10 .......................................... SUCCESS [  4.943 s]
[INFO] ADAM_2.10: Core .................................... FAILURE [ 37.906 s]
[INFO] ADAM_2.10: APIs for Java ........................... SKIPPED
[INFO] ADAM_2.10: CLI ..................................... SKIPPED
[INFO] ADAM_2.10: Assembly ................................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE

As a control, the earlier commit b903cfd
does compile fine for me

@ryan-williams ryan-williams referenced this issue Oct 8, 2016

Merged

fix build #1201

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 8, 2016

Member

Oh drat! Thanks for catching this @jpdna and thanks for the quick fix @ryan-williams. We merged a PR without retesting it after a previous PR merged. Good to be aware of this error model...

Member

fnothaft commented Oct 8, 2016

Oh drat! Thanks for catching this @jpdna and thanks for the quick fix @ryan-williams. We merged a PR without retesting it after a previous PR merged. Good to be aware of this error model...

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Oct 8, 2016

Member

Funny, #1198 added a new call of a method (resourceFile) that was removed by #1142.

We didn't notice because GitHub/git just do a relatively simple lexical pass to determine whether two commits conflict, and this case demonstrates a pretty reliable way to get that algorithm to generate false-negatives for conflicts, resulting in build breaks like this.

I sent the (trivial) fix in #1201.

<rant> The longer term fix is to live in a world where programming tools exist that interpret commits at the AST level or higher, so that "remove resourcePath and reroute all calls to testFile" would be automatically inferred as conflicting with a commit that included new calls to resourcePath (or automatically resolved correctly), but alas, that's probably a few years out! </rant>

Member

ryan-williams commented Oct 8, 2016

Funny, #1198 added a new call of a method (resourceFile) that was removed by #1142.

We didn't notice because GitHub/git just do a relatively simple lexical pass to determine whether two commits conflict, and this case demonstrates a pretty reliable way to get that algorithm to generate false-negatives for conflicts, resulting in build breaks like this.

I sent the (trivial) fix in #1201.

<rant> The longer term fix is to live in a world where programming tools exist that interpret commits at the AST level or higher, so that "remove resourcePath and reroute all calls to testFile" would be automatically inferred as conflicting with a commit that included new calls to resourcePath (or automatically resolved correctly), but alas, that's probably a few years out! </rant>

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 8, 2016

Member

The longer term fix is to live in a world where programming tools exist that interpret commits at the AST level or higher, so that "remove resourcePath and reroute all calls to testFile" would be automatically inferred as conflicting with a commit that included new calls to resourcePath (or automatically resolved correctly), but alas, that's probably a few years out!

That would be pretty fantastic!

Member

fnothaft commented Oct 8, 2016

The longer term fix is to live in a world where programming tools exist that interpret commits at the AST level or higher, so that "remove resourcePath and reroute all calls to testFile" would be automatically inferred as conflicting with a commit that included new calls to resourcePath (or automatically resolved correctly), but alas, that's probably a few years out!

That would be pretty fantastic!

@fnothaft fnothaft closed this in #1201 Oct 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment