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

rustfmt overwriting other files with contents of target file #1229

Closed
jonhoo opened this issue Nov 25, 2016 · 13 comments
Closed

rustfmt overwriting other files with contents of target file #1229

jonhoo opened this issue Nov 25, 2016 · 13 comments
Labels
bug Panic, non-idempotency, invalid code, etc. p-high

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Nov 25, 2016

After upgrading to d3eba76e4d63c08fa0c4406745d1f0cc0e576758, I'm seeing some truly bizarre (and quite disturbing) behavior of rustfmt. Specifically, when I format a particular file in my project, rustfmt overwrites a bunch of other files with the contents of that file! A console example will likely be most helpful:

$ rustfmt --version
0.6.3 (d3eba76)
$ cargo b
   Compiling xxx v0.1.0 (file:///home/jon/dev/projects/xxx)
    Finished debug [unoptimized + debuginfo] target(s) in 3.97 secs
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working tree clean
$ diff -q benchmarks/targets/memcached.rs benchmarks/vote.rs 
Files benchmarks/targets/memcached.rs and benchmarks/vote.rs differ
$ rustfmt benchmarks/vote.rs 
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   benchmarks/targets/memcached.rs

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	benchmarks/targets/memcached.rs.bk

no changes added to commit (use "git add" and/or "git commit -a")
$ diff -q benchmarks/targets/memcached.rs benchmarks/vote.rs
$

This is a somewhat large codebase that isn't public (yet), and so I unfortunately can't point you at the source. The basic structure is as follows though:

$ cat Cargo.toml 
...
[features]
b_memcached = ["memcache"]
...
[lib]
name = "xxx"
path = "src/lib.rs"
...
[[bin]]
name = "vote"
path = "benchmarks/vote.rs"
$ ls -laR benchmarks/
drwxr-xr-x 1 jon users    92 Nov 25 12:56 targets/
-rw-r--r-- 1 jon users 10598 Nov 25 12:53 vote.rs

benchmarks/targets:
-rw-r--r-- 1 jon users 1900 Nov 25 12:56 memcached.rs
-rw-r--r-- 1 jon users  524 Nov 25 12:47 mod.rs
$ grep -EB1 '^mod ' benchmarks/vote.rs 

mod targets;
$ grep -EB1 '^pub mod ' benchmarks/targets/mod.rs 
...
#[cfg(feature="b_memcached")]
pub mod memcached;

The other files that are overwritten are all of the same form as benchmarks/targets/memcached.rs (i.e., they are compiled under a #[cfg(feature = ...)] directive). I have other modules that work the same way, but that are not conditionally compiled, and they are not overwritten by rustfmt.

@jonhoo
Copy link
Contributor Author

jonhoo commented Nov 25, 2016

git bisect shows that d3eba76 introduced the bug (which isn't great given the size of that commit)

@nrc nrc added bug Panic, non-idempotency, invalid code, etc. p-high labels Nov 25, 2016
@nrc
Copy link
Member

nrc commented Nov 25, 2016

Thanks for the bug report. If you could make a minimal test case that reproduces the behaviour that would be extremely helpful. Do you get the same behaviour running rustfmt on other files, or only that one (or some subset of files)?

@jonhoo
Copy link
Contributor Author

jonhoo commented Nov 25, 2016

$ ls -R
.:
foo.rs  targets/

./targets:
bar.rs  mod.rs
$ cat foo.rs 
mod targets;

fn main() {}
$ cat targets/mod.rs 
#[cfg(feature="bar")]
pub mod bar;
$ cat targets/bar.rs 
fn lib() {}
$ rustfmt foo.rs 
$ cat targets/bar.rs 
mod targets;

fn main() {}

@jonhoo
Copy link
Contributor Author

jonhoo commented Nov 25, 2016

Re your question: I remember seeing it on one other file, but can't remember what that was. I thought it was just a fluke at the time, so did a swift git checkout -p.

@jonhoo
Copy link
Contributor Author

jonhoo commented Nov 25, 2016

Note also that this does not trigger if the mod statement with the cfg directive is included directly in foo.rs. The indirection through targets/mod.rs is necessary.

@jonhoo
Copy link
Contributor Author

jonhoo commented Dec 8, 2016

I keep forgetting this, and am constantly finding myself having to do a git checkout targets/*.rs (which is, in and of itself, upsetting, as it may overwrite changes). Can anyone else reproduce the bug with my example above?

@mathstuf
Copy link
Contributor

I am able to reproduce the error here. Using rustfmt as of 56469a8.

@jonhoo
Copy link
Contributor Author

jonhoo commented Jan 17, 2017

Still present in 0.7.0

@nrc
Copy link
Member

nrc commented Jan 17, 2017

I investigated this today, it is due to cfg'ed modules being stripped in the parser (rust-lang/rust#36482). This will need a fix in rustc and that will need to propagate to Syntex before it can have an effect in Rustfmt, but hopefully a fix should be coming up soon-ish. In the meantime, work-around is to ensure all cfgs resolve to true, or comment them out.

@jonhoo
Copy link
Contributor Author

jonhoo commented Jan 17, 2017

@nrc Ah, good catch! That's a pretty painful work-around, but I guess there's not that much we can do about it until a fix percolates down the chain.

@nrc
Copy link
Member

nrc commented Jan 18, 2017

cc rust-lang/rust#39145

@dtolnay
Copy link
Member

dtolnay commented Jan 18, 2017

Syntex 0.56.0 contains the fix from rust-lang/rust#39145.

@jonhoo
Copy link
Contributor Author

jonhoo commented Jan 19, 2017

@nrc I tried just bumping syntex to 0.56.2 and build rustfmt locally to see if the problem would be fixed that way, but (perhaps obviously) it didn't build due to various type changes. I guess we're a decent number of commits behind syntex (0.52 vs 0.56), so this isn't too surprising.

@nrc nrc closed this as completed in e018712 Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc. p-high
Projects
None yet
Development

No branches or pull requests

4 participants