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

pants changed missed a target changed by a deleted file #4529

Closed
JieGhost opened this issue Apr 28, 2017 · 1 comment
Closed

pants changed missed a target changed by a deleted file #4529

JieGhost opened this issue Apr 28, 2017 · 1 comment
Assignees
Labels
Milestone

Comments

@JieGhost
Copy link
Contributor

"./pants changed" won't print a target if the target is changed by deleting a source file.
It was introduced in #4350.

@JieGhost JieGhost added this to the 1.3.0 milestone Apr 28, 2017
@JieGhost JieGhost self-assigned this Apr 28, 2017
JieGhost added a commit that referenced this issue May 4, 2017
### Problem

#4529 

There are 2 kinds of "changed" tasks in pants currently. Neither of them output correct result if a file is deleted.
1. "./pants changed" tasks. They used to have the correct output, but after #4350 , the logic of file matching in SpecSourceMapper was changed. We used to do a regular expression matching to check if the modified file is owned by a target. Now we check if a modified file is in a target's source file set. For a deleted file, it will not be in any target's source file set, thus no target will be considered as the owner of the deleted file, giving incorrect result.
2. "./pants --changed-XXX list". This uses EngineSourceMapper which implements similar logic as above. It never works on deleted files.

### Solution

I restored the removed file matching logic (using regular expression). In both SpecSourceMapper and EngineSourceMapper, first check if a file exists. It it exists, then check if it's in target's file set. If not, then it is a deleted file, and use re matching logic on it. Fixes #4529
@JieGhost
Copy link
Contributor Author

JieGhost commented May 4, 2017

merged in #4546

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

No branches or pull requests

1 participant