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

Add config option to display local project dir in output #184

Closed

Conversation

tony-root
Copy link

The actual command run on remote may sometimes contain absolute references to files, compiler warnings and errors being the most common case.
These references are often clickable (in IDEA for example), but as remote project directory does not match the local one, a click fails to resolve the actual file.

The introduced config option local_dir_in_output will, when enabled, replace all occurrences of the REMOTE_PROJECT_DIR to LOCAL_PROJECT_DIR. This will make the file links in the output clickable.

@artem-zinnatullin @ming13 , PTAL, if you are OK with the idea, I will than update README as well.


if ssh "$REMOTE_MACHINE" "echo 'set -e && cd '$PROJECT_DIR_ON_REMOTE_MACHINE' && echo \"$REMOTE_COMMAND\" && echo "" && $REMOTE_COMMAND' | bash" \
2> >(sed -l "s|$REMOTE_PROJECT_DIR_ABSOLUTE|$PROJECT_DIR|g" >&2) \
1> >(sed -l "s|$REMOTE_PROJECT_DIR_ABSOLUTE|$PROJECT_DIR|g" >&1); then
Copy link
Author

Choose a reason for hiding this comment

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

Both stdin and stderr are processed without redirecting stderr to stdin to preserve the stderr coloring.


if ssh "$REMOTE_MACHINE" "echo 'set -e && cd '$PROJECT_DIR_ON_REMOTE_MACHINE' && echo \"$REMOTE_COMMAND\" && echo "" && $REMOTE_COMMAND' | bash" \
2> >(sed -l "s|$REMOTE_PROJECT_DIR_ABSOLUTE|$PROJECT_DIR|g" >&2) \
1> >(sed -l "s|$REMOTE_PROJECT_DIR_ABSOLUTE|$PROJECT_DIR|g" >&1); then
Copy link
Author

Choose a reason for hiding this comment

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

-l is important to process the output line-by-line instead of waiting for the remote build to complete

Copy link
Contributor

@dmitry-novikov dmitry-novikov left a comment

Choose a reason for hiding this comment

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

Very nice!

@tony-root
Copy link
Author

tony-root commented Feb 8, 2018

Hi @artem-zinnatullin, @ming13,

Looks like the Travis CI build process generates over 4M logs and gets shut down because of it. Do you have an idea on where to raise the 4M logs? Any other suggestions?

https://travis-ci.org/gojuno/mainframer/builds/322881594

...
Info: Preparing "Install Android SDK Platform 25 (revision: 3)".
[====================                   ] 51% Unzipping... android-7.1.1/data/la
The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over).
The job has been terminated

@skrugly skrugly mentioned this pull request Feb 13, 2018
@artem-zinnatullin
Copy link
Contributor

@AntonRutkevich there is no good workaround for 4mb limit on Travis other than reducing amount of logs we produce during build :( (faced this in StorIO)

Btw, if your use case happens in IntelliJ - Mainframer IntelliJ Plugin handles path mapping, do you use it?

@tony-root
Copy link
Author

@artem-zinnatullin

I am indeed using the Mainframer IntelliJ plugin.
What kind of path mapping are you referring to? What I'm trying to achieve is to be able to click the links in the compiler warnings/erros output inside IntelliJ. I could not find such setting in the Mainframer plugin readme, settings etc.

@tony-root
Copy link
Author

Also, regarding the build output: as you see it's not something introduced by this PR, it's about environment setup, so I assume that no PR in this repo will be able to build successfully..

@artem-zinnatullin
Copy link
Contributor

I'll be checking Mainframer CI soon

Here are few issues in Mainframer IntelliJ Plugin regarding it:

I don't have time to check plugin sources, but it might be that they only apply mapping in case of error, better double check with them and open an issue there if that's the case :)

@tony-root
Copy link
Author

@artem-zinnatullin
Thanks, I'll check with them. Although I'm still not sure if it's better to keep it as a part of plugin or mainframer itself.

Have a good weekend :)

@artem-zinnatullin
Copy link
Contributor

Yep, I agree, this might be a great addition to Mainframer itself

Have a good weekend man!

@artem-zinnatullin
Copy link
Contributor

@AntonRutkevich wanna rewrite this in Rust :trollface:?

If you could add integration tests in this PR, I'd be happy to merge so later I could reuse them for Rust implementation :)

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

Successfully merging this pull request may close these issues.

3 participants