Skip to content

syntax: Remove diagnostic validation temporarily #25592

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

Closed

Conversation

alexcrichton
Copy link
Member

This commit removes the syntax diagnostic validation as it's been plauging the
bots for two separate reasons and needs a quick fix for now:

  1. It's possible to read the stale metadata of previous builds, causing spurious
    failures that can only be solved by blowing away tmp.
  2. The code does not currently handle concurrent compilations where JSON files
    in the metadata directory may be half-written and hence contain invalid JSON
    when read.

This validation should come back, but it should likely be part of a lint pass
run at the very end of the build that doesn't run into consistency or
concurrency problems.

This commit removes the syntax diagnostic validation as it's been plauging the
bots for two separate reasons and needs a quick fix for now:

1. It's possible to read the stale metadata of previous builds, causing spurious
   failures that can only be solved by blowing away `tmp`.
2. The code does not currently handle concurrent compilations where JSON files
   in the metadata directory may be half-written and hence contain invalid JSON
   when read.

This validation should come back, but it should likely be part of a lint pass
run at the very end of the build that doesn't run into consistency or
concurrency problems.
@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

cc @pnkfelix, @michaelsproul, #25380

@Manishearth
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented May 19, 2015

📌 Commit 7313a55 has been approved by Manishearth

@michaelsproul
Copy link
Contributor

Perhaps just remove the call to check_uniqueness, as the error index page depends on the JSON.

@michaelsproul
Copy link
Contributor

Concurrent writing to separate files is fine, we just have to get rid of the unsynchronised reads.

Longer-term I agree that the uniqueness checking isn't particularly important, but it could be done in a Python script that runs once all the JSON files are generated (as you noted in the other thread, Alex).

@pnkfelix
Copy link
Member

@michaelsproul I agree. I'll r- this and put up an alternative PR that keeps the generation and just removes the uniqueness checking.

(Normally I'd let this land, but there are enough entries in the queue that I think I have time to come up with the alternative.)

@pnkfelix
Copy link
Member

@bors r-

pnkfelix added a commit to pnkfelix/rust that referenced this pull request May 19, 2015
…guing the

bots for two separate reasons and needs a quick fix for now:

1. It's possible to read the stale metadata of previous builds, causing spurious
   failures that can only be solved by blowing away `tmp`.
2. The code does not currently handle concurrent compilations where JSON files
   in the metadata directory may be half-written and hence contain invalid JSON
   when read.

This validation should come back, but it should likely be part of a lint pass
run at the very end of the build that doesn't run into consistency or
concurrency problems.

----

This commit is a revised version of PR rust-lang#25592; this approach differs
in that it continues to output the JSON metadata, so that the error
index page continues to work (assuming that people do not accidentally
put in duplicate entries in the meantime).
@pnkfelix
Copy link
Member

closing in favor of PR #25603

@pnkfelix pnkfelix closed this May 19, 2015
pnkfelix added a commit to pnkfelix/rust that referenced this pull request May 21, 2015
…guing the

bots for two separate reasons and needs a quick fix for now:

1. It's possible to read the stale metadata of previous builds, causing spurious
   failures that can only be solved by blowing away `tmp`.
2. The code does not currently handle concurrent compilations where JSON files
   in the metadata directory may be half-written and hence contain invalid JSON
   when read.

This validation should come back, but it should likely be part of a lint pass
run at the very end of the build that doesn't run into consistency or
concurrency problems.

----

This commit is a revised version of PR rust-lang#25592; this approach differs
in that it continues to output the JSON metadata, so that the error
index page continues to work (assuming that people do not accidentally
put in duplicate entries in the meantime).
@alexcrichton alexcrichton deleted the remove-json-validation branch July 10, 2015 22:32
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.

6 participants