Skip to content
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

Update tasks after build #933

Merged
merged 4 commits into from Apr 24, 2015
Merged

Conversation

Kwestor
Copy link
Member

@Kwestor Kwestor commented Apr 24, 2015

During each build tasks are updated for every new and/or updated file.

Fixes #1002137

Lastly, the below disclaimer is required by the lawyers:

THE FOLLOWING DISCLAIMER APPLIES TO ALL SOFTWARE CODE AND OTHER MATERIALS CONTRIBUTED IN CONNECTION WITH THIS SOFTWARE:
THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS “AS IS” AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.
ONLY THE SOFTWARE CODE AND OTHER MATERIALS CONTRIBUTED IN CONNECTION WITH THIS SOFTWARE, IF ANY, THAT ARE ATTACHED TO (OR OTHERWISE ACCOMPANY) THIS SUBMISSION (AND ORDINARY COURSE CONTRIBUTIONS OF FUTURES PATCHES THERETO) ARE TO BE CONSIDERED A CONTRIBUTION. NO OTHER SOFTWARE CODE OR MATERIALS ARE A CONTRIBUTION.

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scala-ide-multi-conf/88/
Test PASSed.

@mpociecha
Copy link
Contributor

Basically LGTM (you could change some names like mrk to marker but it doesn't matter)

And it's a question how it works in case of really big projects. What is the additional time needed to perform this?

val length = taskPos.end - start
taskPos.source.file match {
case EclipseResource(i: IFile) =>
registerTask(i, tag, msg, priority, start, length, taskPos.line)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrapRegisterTask in method that takes 2 arguments i (btw rename i to some more meaningfull)

question to all: Can't we use iFile from above?


private def extractComments(iFile: IFile): Seq[Comment] = {
val scalaFile = ScalaSourceFile.createFromPath(iFile.getFullPath.toOSString)
.getOrElse(return Seq.empty) // TODO - handle java files?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return in by name param? I don't think it is good idea

During each build tasks are updated for every new and/or updated file.

Fixes #1002137
@Kwestor
Copy link
Member Author

Kwestor commented Apr 24, 2015

return removed, some other changes (mostly renames) applied.

I also moved clearTasks method from FileUtils to TaskManager to keep logic for adding and removing markers in the same place.

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scala-ide-multi-conf/93/
Test PASSed.

@dragos
Copy link
Member

dragos commented Apr 24, 2015

I think there was a test for this functionality. Could you locate it and un-@Ignore it?

@dragos
Copy link
Member

dragos commented Apr 24, 2015

otherwise LGTM

@kiritsuku
Copy link
Member

LGTM

@kiritsuku
Copy link
Member

The test is org.scalaide.core.sbtbuilder.TodoBuilderTest.

Code cleanup plus additional test for `FIXME`.
@Kwestor
Copy link
Member Author

Kwestor commented Apr 24, 2015

Test re-enabled and re-written (it was kind of bleh). Do you think we can now close SI-7363 now?

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scala-ide-multi-conf/96/

Build result: FAILURE

GitHub pull request #933 of commit cd8794d automatically merged.Setting status of cd8794d to PENDING with url https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-scala-ide-multi-conf/96/ and message: Merged build started.Building remotely on typesafe-a1 in workspace /localhome/jenkinside/workspace/ghprb-scala-ide-multi-conf > /usr/bin/git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > /usr/bin/git config remote.origin.url git://github.com/scala-ide/scala-ide.git # timeout=10Fetching upstream changes from git://github.com/scala-ide/scala-ide.git > /usr/bin/git --version # timeout=10 > /usr/bin/git -c core.askpass=true fetch --tags --progress git://github.com/scala-ide/scala-ide.git +refs/heads/:refs/remotes/origin/ > /usr/bin/git rev-parse refs/remotes/origin/master^{commit} # timeout=10 > /usr/bin/git rev-parse refs/remotes/origin/origin/master^{commit} # timeout=10Checking out Revision cb758fa (refs/remotes/origin/master) > /usr/bin/git config core.sparsecheckout # timeout=10 > /usr/bin/git checkout -f cb758fa > /usr/bin/git rev-list ab53de0 # timeout=10Triggering ghprb-scala-ide-multi-conf » eclipse-lunaTriggering ghprb-scala-ide-multi-conf » eclipse-keplerghprb-scala-ide-multi-conf » eclipse-luna completed with result FAILUREghprb-scala-ide-multi-conf » eclipse-kepler completed with result FAILURESetting status of cd8794d to FAILURE with url https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-scala-ide-multi-conf/96/ and message: Merged build finished.
Test FAILed.

@Kwestor
Copy link
Member Author

Kwestor commented Apr 24, 2015

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 26:40.725s
[INFO] Finished at: Fri Apr 24 22:32:34 CEST 2015
[INFO] Final Memory: 143M/658M
[INFO] ------------------------------------------------------------------------
/tmp/hudson1043709263555576595.sh: line 44: syntax error near unexpected token `)'
/tmp/hudson1043709263555576595.sh: line 44: `    echo "An update site with the contents of this PR is available at http://download.scala-ide.org/pr-builds/scala-ide/pr-${pullrequest}/2.11.x/site")'
Build step 'Execute shell' marked build as failure
Recording test results
Finished: FAILURE

@kiritsuku
Copy link
Member

Oh jenkins, why are you so brocken? Luckily, all tests passed. @dragos Investigating this failure is something for you.

kiritsuku added a commit that referenced this pull request Apr 24, 2015
@kiritsuku kiritsuku merged commit 6fcd029 into scala-ide:master Apr 24, 2015
@kiritsuku
Copy link
Member

I just closed SI-7363.

@Kwestor
Copy link
Member Author

Kwestor commented Apr 24, 2015

Thanks!

@Kwestor Kwestor deleted the feature/tasks-1002137 branch April 27, 2015 10:07
@lrytz
Copy link
Contributor

lrytz commented May 4, 2015

Hi @Kwestor

cc @adriaanm @retronym

Over at https://github.com/scala/scala we are experiencing failures in the ide-integration job of our PR-validation since this commit is merged.

The failure is consistent (not flaky). The last successful build is this, which used scala-ide revision cb758fa (HEAD before merging this PR): https://scala-ci.typesafe.com/job/scala-2.11.x-integrate-ide/822/console

Since then, every build failed in the same way (if it didn't fail earlier for some other reason). Here's the first one which builds scala-ide revision 6fcd029 (the merge of this PR): https://scala-ci.typesafe.com/job/scala-2.11.x-integrate-ide/823/console

Tests in error: 
  transitive_dep_indirect(org.scalaide.core.sbtbuilder.ProjectDependenciesTest): Errors occurred during the build.

Sorry we didn't notify you earlier, there were other things in flux with our PR validation due to the move to bintray, and because Adriaan set up a new artifactory repo and maven proxy on our build infrastructure.

I managed to grab the logs of the failing build, i put them into a gist: https://gist.github.com/lrytz/8e31c4e1a064bf004486. Note that the stack trace of the failure [1] goes through the new code path introduced by this PR [2].

Contains: Errors running builder 'Scala Builder' on project 'completion'.
java.nio.charset.MalformedInputException: Input length = 1

[1] https://gist.github.com/lrytz/8e31c4e1a064bf004486#file-org-scalaide-testssuite-txt-L85

[2] https://github.com/scala-ide/scala-ide/pull/933/files#diff-44bc051d84135233f92f85c86cf7aa98R128

@mpociecha
Copy link
Contributor

@lrytz We've seen there are problems on your Jenkins (#936), but we lacked details.Thanks!

@lrytz
Copy link
Contributor

lrytz commented May 4, 2015

Great, you were already looking into this, I didn't see that ticket!

@Kwestor
Copy link
Member Author

Kwestor commented May 4, 2015

@lrytz I don't know if there are an issue for that, but I commited a fix here: #943.

@lrytz
Copy link
Contributor

lrytz commented May 4, 2015

@Kwestor excellent, thanks!

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