Skip to content

Conversation

@rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented Feb 26, 2018

Fixes an issue where multiple projects would try to build the test libraries simultaneously.

Fixes https://github.com/xamarin/maccore/issues/637.

Fixes an issue where multiple projects would try to build the test libraries simultaneously.
@spouliot
Copy link
Contributor

related to any known issue on the bots ?

@rolfbjarne
Copy link
Member Author

@spouliot yes, it fixes https://github.com/xamarin/maccore/issues/637 (I've updated the PR description).

BuildTestLibraries ();
if (!IsServerMode) {
foreach (var task in Tasks)
tasks.Add (task.RunAsync ());
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be wrong here, ProcessHelper.ExecuteCommandAsync does return a task, correct?

public static Task<bool> PollForExitAsync (int pid, TimeSpan timeout)

The task is ignored in BuildTestLibraries that returns void (which we should only do in events). So, I think we should be paying attention to the result of the task and, since it is async but needed, we might wait for it to success before we do anything else (there is now guaranteed AFAIK that the build will be done before we needed it).

I might be completely wrong here, but just wanted you to confirm is correct.

void BuildTestLibraries ()
{
ProcessHelper.ExecuteCommandAsync ("make", $"all -j{Environment.ProcessorCount} -C {StringUtils.Quote (Path.Combine (Harness.RootDirectory, "test-libraries"))}", MainLog, TimeSpan.FromMinutes (1)).Wait ();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should return a Task, not void.

Copy link
Member Author

Choose a reason for hiding this comment

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

I call .Wait () at the end to make it synchronous.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agh, I did not see that, cool...

@monojenkins
Copy link
Collaborator

Build failure

@rolfbjarne
Copy link
Member Author

Test failure is unrelated:

@rolfbjarne rolfbjarne merged commit afb6c6c into dotnet:master Feb 27, 2018
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Feb 28, 2018
dotnet#3601)

Fixes an issue where multiple projects would try to build the test libraries simultaneously.
rolfbjarne added a commit that referenced this pull request Mar 1, 2018
#3601) (#3621)

Fixes an issue where multiple projects would try to build the test libraries simultaneously.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants