Better output for parallel install. #964

Merged
merged 1 commit into from Jul 3, 2012

Conversation

Projects
None yet
3 participants
@23Skidoo
Member

23Skidoo commented Jul 2, 2012

Example of what happens in the normal case: https://gist.github.com/3032723
Example of what happens in case of error: https://gist.github.com/3032939

@tibbe

This comment has been minimized.

Show comment Hide comment
@tibbe

tibbe Jul 2, 2012

Owner

Could you please add some comments to your code, especially to the part that overrides the logging verbosity (e.g. taking the max of two verbosities etc.)

Owner

tibbe commented Jul 2, 2012

Could you please add some comments to your code, especially to the part that overrides the logging verbosity (e.g. taking the max of two verbosities etc.)

@tibbe

This comment has been minimized.

Show comment Hide comment
@tibbe

tibbe Jul 2, 2012

Owner

@dcoutts Could you please have a look at this change to make sure we're doing the right things to suppress build output in the case of parallell builds?

Owner

tibbe commented Jul 2, 2012

@dcoutts Could you please have a look at this change to make sure we're doing the right things to suppress build output in the case of parallell builds?

@23Skidoo

This comment has been minimized.

Show comment Hide comment
@23Skidoo

23Skidoo Jul 2, 2012

Member

Added some comments.

(e.g. taking the max of two verbosities etc.)

This is just code reorganization. When the user has specified --build-log=PATH or --remote-build-reporting=detailed, logging should be more verbose.

Member

23Skidoo commented Jul 2, 2012

Added some comments.

(e.g. taking the max of two verbosities etc.)

This is just code reorganization. When the user has specified --build-log=PATH or --remote-build-reporting=detailed, logging should be more verbose.

Better output for parallel install.
Example of what happens in the normal case: https://gist.github.com/3032723
Example of what happens in case of error: https://gist.github.com/3032939
@dcoutts

This comment has been minimized.

Show comment Hide comment
@dcoutts

dcoutts Jul 2, 2012

Member

The output is certainly nicer. Code looks ok too. I'm happy for this to be merged.

One further improvement might be to do something about the:

cabal: Error: some packages failed to install:
Win32-2.2.2.0 failed during the configure step. The exception was:
ExitFailure 1

If we could combine this with the nice thing about printing the last 10 lines that'd be ideal. Printing the exception is pretty useless where as the last 10 lines of the build log are pretty good, plus the file name if you want to look further. The difficulty with making a further change here is what to do in the normal case when we're doing a normal build logged to the console. In that case we don't collect the log so we would not be able to print the last 10 lines. I think the solution here would be to improve Cabal's process utils so that we can run a program and both log the output to a file and display it live on screen, like the unix tee utility.

Member

dcoutts commented Jul 2, 2012

The output is certainly nicer. Code looks ok too. I'm happy for this to be merged.

One further improvement might be to do something about the:

cabal: Error: some packages failed to install:
Win32-2.2.2.0 failed during the configure step. The exception was:
ExitFailure 1

If we could combine this with the nice thing about printing the last 10 lines that'd be ideal. Printing the exception is pretty useless where as the last 10 lines of the build log are pretty good, plus the file name if you want to look further. The difficulty with making a further change here is what to do in the normal case when we're doing a normal build logged to the console. In that case we don't collect the log so we would not be able to print the last 10 lines. I think the solution here would be to improve Cabal's process utils so that we can run a program and both log the output to a file and display it live on screen, like the unix tee utility.

@23Skidoo

This comment has been minimized.

Show comment Hide comment
@23Skidoo

23Skidoo Jul 3, 2012

Member

Regarding the exception, this is no worse than the current output in the serial case. In the future, I want to implement a dynamically-updated status indicator, like described in the original ticket (#440). We can use it both in the serial and in the parallel case, which will make the code simpler.

Member

23Skidoo commented Jul 3, 2012

Regarding the exception, this is no worse than the current output in the serial case. In the future, I want to implement a dynamically-updated status indicator, like described in the original ticket (#440). We can use it both in the serial and in the parallel case, which will make the code simpler.

@tibbe

This comment has been minimized.

Show comment Hide comment
@tibbe

tibbe Jul 3, 2012

Owner

@23Skidoo Let me know if you want me to merge this as-is or if you want to do any of Duncan's suggested changes now.

Owner

tibbe commented Jul 3, 2012

@23Skidoo Let me know if you want me to merge this as-is or if you want to do any of Duncan's suggested changes now.

@23Skidoo

This comment has been minimized.

Show comment Hide comment
@23Skidoo

23Skidoo Jul 3, 2012

Member

I'd like to merge this change right now; at some later point in the future I'll implement a dynamic status indicator (which will subsume Duncan's suggestion).

Member

23Skidoo commented Jul 3, 2012

I'd like to merge this change right now; at some later point in the future I'll implement a dynamic status indicator (which will subsume Duncan's suggestion).

@tibbe tibbe merged commit a91f71d into haskell:master Jul 3, 2012

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