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

Upgrade zinc to 1.0.0-X7 (python portion) #4419

Merged
merged 11 commits into from
Apr 4, 2017

Conversation

peiyuwang
Copy link
Contributor

@peiyuwang peiyuwang commented Apr 4, 2017

Problem

Notable change in 1.0.0-X7: sbt/zinc@a25d135 to fix zinc swallow javac error messages.

Two zinc TODOs that we should push upstream before next upgrade are:

Solution

Upgrade and workarounds of these two TODOs, see comments inline.

Result

Passed travis

@peiyuwang peiyuwang requested review from stuhood and benjyw April 4, 2017 00:48
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks Peiyu.

val = base64.b64decode(val)
for rebased_from, rebased_to in rebase_mappings:
val = val.replace(rebased_from, rebased_to)
rebased_line = '{}{}\n'.format(prefix, base64.b64encode(val))
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Should be possible to skip re-encoding if no changes were made? Could set a variable inside the loop to mark a change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebase_pants_home_anywhere_base64 flag means lines in this section contains b64 encoded paths, so detecting if replace happens is unnecessary imo. Same is true for other flags like rebase_pants_home_anywhere or rebase_pants_home_prefix, i.e, once set no further optimization is needed.

@peiyuwang peiyuwang merged commit 8b6adf2 into pantsbuild:master Apr 4, 2017
baroquebobcat added a commit to twitter/pants that referenced this pull request Apr 12, 2017
stuhood added a commit to twitter/pants that referenced this pull request Apr 17, 2017
stuhood added a commit to twitter/pants that referenced this pull request Apr 24, 2017
stuhood pushed a commit that referenced this pull request Apr 24, 2017
### Problem

The zinc 1.0.0-X7 upgrade resulted in a few issues, including complete target invalidation (#4477), and some unconfirmed assertion failures from within zinc.

### Solution

This reverts #4419 until after the `1.3.0` stable branch has been cut. It should be re-landed immediately afterward.
lenucksi pushed a commit to lenucksi/pants that referenced this pull request Apr 25, 2017
### Problem

Notable change in `1.0.0-X7`: sbt/zinc@a25d135 to fix zinc swallow javac error messages.

Two zinc `TODO`s that we should push upstream before next upgrade are:

* sbt/util#75 to generate machine independent analysis file. So we don't have to do the extra work in this PR decode/encode `classpath hash`.
* sbt/zinc#218 jdk 7 compatibility. If zinc decide to discontinue jdk7 support we should start to communicate with pants users. Since the incompatibility is already introduced, this PR has to exclude the jdk8 compiled deps.

### Solution

Upgrade and workarounds of these two TODOs, see comments inline.

### Result

Passed travis
lenucksi pushed a commit to lenucksi/pants that referenced this pull request Apr 25, 2017
### Problem

The zinc 1.0.0-X7 upgrade resulted in a few issues, including complete target invalidation (pantsbuild#4477), and some unconfirmed assertion failures from within zinc.

### Solution

This reverts pantsbuild#4419 until after the `1.3.0` stable branch has been cut. It should be re-landed immediately afterward.
thesamet pushed a commit to thesamet/pants that referenced this pull request May 9, 2017
### Problem

The zinc 1.0.0-X7 upgrade resulted in a few issues, including complete target invalidation (pantsbuild#4477), and some unconfirmed assertion failures from within zinc.

### Solution

This reverts pantsbuild#4419 until after the `1.3.0` stable branch has been cut. It should be re-landed immediately afterward.
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.

2 participants