Issue 4503 - ensure_dependencies.py fails for git submodule

Issue 4503 - ensure_dependencies.py fails for git submodule

[anton, 2016-10-07 19:11:44]

https://codereview.adblockplus.org/29356341/diff/29356342/ensure_dependencies.py
File ensure_dependencies.py (right):

https://codereview.adblockplus.org/29356341/diff/29356342/ensure_dependencies...
ensure_dependencies.py:80: _ensure_line_exists(ignore_path, module)
probably should add try except here too

[Felix Dahlke, 2016-10-11 10:30:15]

Please note that ensure_dependencies.py is being pulled from buildtools, so this
has to be addressed there.

Sebastian (sebastian@adblockplus.org) is module owner for buildtools, could you
make him reviewer and put Wladimir (trev@adblockplus.org) in CC? I'm not really
involved with buildtools, so I should just be in CC if anything.

[Sebastian Noack, 2016-10-11 11:59:21]

So as far as I understand, if in a git submodule, .git is not the actual git dir
but a file that refers to the location of the actual git dir:

  gitdir: ../.git/modules/libadblockplus-android

So I guess, if my observations is right, we should read that file and then
modify ../.git/modules/libadblockplus-android/info/exclude instead.

[anton, 2016-10-11 12:20:47]

On 2016/10/11 11:59:21, Sebastian Noack wrote:
> So as far as I understand, if in a git submodule, .git is not the actual git
dir
> but a file that refers to the location of the actual git dir:
> 
>   gitdir: ../.git/modules/libadblockplus-android
> 
> So I guess, if my observations is right, we should read that file and then
> modify ../.git/modules/libadblockplus-android/info/exclude instead.

Sounds reasonable, what about Mercurial Subrepositories?
Should i continue working on it or you?

[Sebastian Noack, 2016-10-11 12:23:57]

On 2016/10/11 12:20:47, anton wrote:
> On 2016/10/11 11:59:21, Sebastian Noack wrote:
> > So as far as I understand, if in a git submodule, .git is not the actual git
> dir
> > but a file that refers to the location of the actual git dir:
> > 
> >   gitdir: ../.git/modules/libadblockplus-android
> > 
> > So I guess, if my observations is right, we should read that file and then
> > modify ../.git/modules/libadblockplus-android/info/exclude instead.
> 
> Sounds reasonable, what about Mercurial Subrepositories?
> Should i continue working on it or you?

I think if necessary we can address mercurial subrepos in a different issue. I
don't even know whether we have the same issue there.

[anton, 2016-10-11 13:03:25]

On 2016/10/11 12:23:57, Sebastian Noack wrote:
> On 2016/10/11 12:20:47, anton wrote:
> > On 2016/10/11 11:59:21, Sebastian Noack wrote:
> > > So as far as I understand, if in a git submodule, .git is not the actual
git
> > dir
> > > but a file that refers to the location of the actual git dir:
> > > 
> > >   gitdir: ../.git/modules/libadblockplus-android
> > > 
> > > So I guess, if my observations is right, we should read that file and then
> > > modify ../.git/modules/libadblockplus-android/info/exclude instead.
> > 
> > Sounds reasonable, what about Mercurial Subrepositories?
> > Should i continue working on it or you?
> 
> I think if necessary we can address mercurial subrepos in a different issue. I
> don't even know whether we have the same issue there.

Should i continue working on it or you?

[Sebastian Noack, 2016-10-11 13:39:51]

On 2016/10/11 13:03:25, anton wrote:
> On 2016/10/11 12:23:57, Sebastian Noack wrote:
> > On 2016/10/11 12:20:47, anton wrote:
> > > On 2016/10/11 11:59:21, Sebastian Noack wrote:
> > > > So as far as I understand, if in a git submodule, .git is not the actual
> git
> > > dir
> > > > but a file that refers to the location of the actual git dir:
> > > > 
> > > >   gitdir: ../.git/modules/libadblockplus-android
> > > > 
> > > > So I guess, if my observations is right, we should read that file and
then
> > > > modify ../.git/modules/libadblockplus-android/info/exclude instead.
> > > 
> > > Sounds reasonable, what about Mercurial Subrepositories?
> > > Should i continue working on it or you?
> > 
> > I think if necessary we can address mercurial subrepos in a different issue.
I
> > don't even know whether we have the same issue there.
> 
> Should i continue working on it or you?

Well, I won't work on this any time soon (but perhaps somebody else). If you
want to address the comments yourself feel free to go ahead, otherwise unassign
yourself from the issue.

[Wladimir Palant, 2016-10-11 14:24:17]

On 2016/10/11 12:20:47, anton wrote:
> Sounds reasonable, what about Mercurial Subrepositories?

What about them? Mercurial subrepositories are no different than "regular"
repositories, no special treatment should be necessary.

You are patching an outdated version of ensure_repositories.py, this isn't even
the current state of the libadblockplus repository. Please change the master
copy in the buildtools repository, then propagate it via dependency updates
(libadblockplus doesn't depend on buildtools directly so this requires updating
the dependency in adblockpluscore first, then updating adblockpluscore
dependencies in libadblockplus).

[anton, 2016-10-14 13:14:14]

On 2016/10/11 14:24:17, Wladimir Palant wrote:
> On 2016/10/11 12:20:47, anton wrote:
> > Sounds reasonable, what about Mercurial Subrepositories?
> 
> What about them? Mercurial subrepositories are no different than "regular"
> repositories, no special treatment should be necessary.
> 
> You are patching an outdated version of ensure_repositories.py, this isn't
even
> the current state of the libadblockplus repository. Please change the master
> copy in the buildtools repository, then propagate it via dependency updates
> (libadblockplus doesn't depend on buildtools directly so this requires
updating
> the dependency in adblockpluscore first, then updating adblockpluscore
> dependencies in libadblockplus).

i've replaced `ensure_dependencies.py` with the latest version from `buildtools`
repository and i can confirm the issue remains.
to be fixed properly (with Sebastian's solution) later.