Opened on 10/25/2016 at 08:00:58 PM
Closed on 10/26/2016 at 07:41:42 PM
Last modified on 12/07/2016 at 02:49:23 PM
#4569 closed defect (fixed)
dependencies are not ignored when using git due to ensure_dependencies.py
| Reported by: | erikvold | Assignee: | trev |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Automation | Keywords: | |
| Cc: | trev, kzar | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by trev)
How to reproduce
- git clone https://github.com/adblockplus/adblockplus newdir
- ./build.py build
- git status
Observed behaviour
Multiple dependencies which ensure_dependencies.py has downloaded are not ignored, and are listed as changes when using git status.
Expected behaviour
All of the dependencies which ensure_dependencies.py has downloaded are ignored, and are not listed as changes when using git status.
Summary
Basically I think that https://github.com/adblockplus/adblockplus/blob/master/ensure_dependencies.py#L129-L132 has the issue somewhere.
I wonder if we really need this code though, I think we will constantly run into bugs or git/hg updates that need addressing which will cause this code to break. Would it not just be easier to explicitly list the dependences as folders to ignore in ignore files? and remove this code.
Background
Function _ensure_line_exists() always removes all previous content from the file instead of merely adding a line. It doesn't seek before reading file's contents, so it attempts reading from the end of the file. As a result, it doesn't read anything.
Attachments (0)
Change History (12)
comment:1 Changed on 10/25/2016 at 08:28:39 PM by trev
- Cc trev kzar added
comment:2 Changed on 10/26/2016 at 01:51:14 PM by kzar
comment:3 Changed on 10/26/2016 at 06:09:02 PM by trev
I can reproduce that on Mac OS X with Git 2.9.2 and Python 2.7.12. git status command shows adblockpluscore/ and buildtools/ as untracked but not adblockplusui/. The file system is case insensitive but that's not the reason - .git/info/exclude file only lists /adblockplusui but not the other dependencies.
comment:4 Changed on 10/26/2016 at 06:17:03 PM by trev
- Description modified (diff)
- Owner set to trev
I identified the bug in _ensure_line_exists() function, added the analysis to the description.
comment:5 Changed on 10/26/2016 at 06:19:36 PM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:6 Changed on 10/26/2016 at 07:33:13 PM by kzar
It's weird this bug didn't affect me, but your explanation and solution makes sense. (I even tested opening a file in "a+" mode and found the handle pointed to the start of the file. I guess my version of Python just behaves differently here.)
comment:7 Changed on 10/26/2016 at 07:41:06 PM by trev
- Ready set
@kzar: Given that you approved my patch, I guess that you consider this issue ready ;)
comment:8 Changed on 10/26/2016 at 07:41:20 PM by abpbot
A commit referencing this issue has landed:
Issue 4569 - Make sure to add ignored directories without overwriting existing file content
comment:9 Changed on 10/26/2016 at 07:41:42 PM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
comment:10 Changed on 10/26/2016 at 07:43:32 PM by kzar
- Priority changed from Unknown to P2
comment:11 Changed on 12/07/2016 at 02:24:59 PM by wspee
- Blocking 4715 added
comment:12 Changed on 12/07/2016 at 02:49:23 PM by wspee
- Blocking 4715 removed
I can't reproduce this as described (I'm assuming step 1a. is cd newdir), the dependencies are ignored by Git as expected. What version of Git and Python do you have? (I have Git 2.9.3 and Python 2.7.12+.) Also just a hunch but are you using a case insensitive filesystem?