Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#4584 closed change (fixed)

ensure_dependencies: Ignore dependencies in Mercurial unconditionally

Reported by: trev Assignee: kzar
Priority: P3 Milestone:
Module: Automation Keywords:
Cc: fhd Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29360367/
https://codereview.adblockplus.org/29360531/

Description

Background

ensure_dependencies.py will add dependencies to the ignore file automatically, so that they don't appear in the status command. This is unconditional for Git, for Mercurial however we only do it if the dependency is a Git repository (Mercurial will ignore other Mercurial repositories automatically). It seems that fsmonitor extension doesn't follow the same logic however and won't ignore Mercurial repositories automatically.

What to change

Make sure to set a relative rather than absolute path to the generated ignore file (supported as of Mercurial 3.2.1). Add dependencies to that file unconditionally, regardless of repository type.

Change History (12)

comment:1 Changed 3 years ago by fhd

  • Cc fhd added

Maybe we should add a check for the Mercurial version then, to avoid mysterious issues?

comment:2 follow-up: Changed 3 years ago by kzar

  • Owner set to kzar
  • Priority changed from Unknown to P3
  • Ready set

Well it seems kind of unlikely someone would have a version of Mercurial that's more than two years old.

comment:3 Changed 3 years ago by trev

I thought that maybe ensure_dependencies.py could prompt people to update Mercurial (definitely nothing beyond that) but even this seems to be non-trivial. Mercurial won't produce a clean version number, even the output of hg version --quiet needs to be parsed. IMHO that's too much effort.

comment:4 in reply to: ↑ 2 Changed 3 years ago by trev

Replying to kzar:

Well it seems kind of unlikely someone would have a version of Mercurial that's more than two years old.

Actually, server16 (the server creating development builds, so executing ensure_dependencies.py as part of the process) runs a version of Mercurial older than 3.2.1. Not that it would run into issues because of it, it never runs hg stat...

comment:5 Changed 3 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:6 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4584 - Ignore Mercurial dependencies explicitly

comment:7 Changed 3 years ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

There you go Felix. (FYI dependencies are only ignored at the time that they're cloned by ensure_dependencies.py, so if you already have them cloned the script won't ignore them.)

comment:8 Changed 3 years ago by fhd

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks, works like a charm for the adblockbrowser repo. But there appears to be an issue with subrepos, in adblockbrowser/buildtools I get this:

$ hg status
skipping unreadable pattern file '/Users/fhd/Projects/adblockplus/adblockbrowser/buildtools/./buildtools/.hg/dependencies': No such file or directory

And indeed, the ignore.dependencies path seems wrong:

$ cat .hg/hgrc 
[paths]
default = https://hg.adblockplus.org/buildtools/

[ui]
ignore.dependencies = ./buildtools/.hg/dependencies

Should be just .hg/dependencies, or ./.hg/dependencies since that's the general pattern here.

comment:9 Changed 3 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

Dang you're right. Sorry, I should have spotted this sooner.

I suspected this code never ran previously since I couldn't see how it ever could, now I suspect it never even worked previously either!

Edit: On second thought I realise I was wrong, since we used the absolute path previously it would have worked.

Last edited 3 years ago by kzar (previous) (diff)

comment:10 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4584 - Fix ignore file path

comment:11 Changed 3 years ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

Should work properly this time!

comment:12 Changed 3 years ago by fhd

  • Verified working set

Yup, works just fine now!

Note: See TracTickets for help on using tickets.