celery: fix CELERY_APP creation after breakage in ea1c608efa3a

A part of change still under development had sneaked in.

vcs: fix subprocessio process termination

659ecd26002c had a cut'n'paste error so it dropped process termination in the
SubprocessIOChunker close method.

lib: clean up pointless use of h.person on db.User just to get .username

After cleaning up, h.person will never be invoked with a db.User .

indexer: drop "akc" cleanup

It was introduced in 0eceb478c720 but it is unclear what it does. We would
rather understand and fix any js encoding errors.

vcs: drop repo contact ... and the odd uses of it

A half-baked and essentially unused hgweb feature.

The reference in get_commits_stats doesn't seem reachable in any relevant use
cases.

lib: move urlification to webutils

Less use of helpers in model.

notifications: only render body with mentions if there is a body

Notifications for registering a user would pass body=None. It is unfortunate to
pass None all the way to render_w_mentions - it seems fair enough that this
function expects a str.

lib: drop is_hg and is_git

It is just as simple to be explicit.

templates: pass repo type to repolabel as str

Keep it simple and avoid passing complex repo objects around.

lib: move js escaping from helpers to webutils

It was no real problem it lived in helpers: it was only used from templates
anyway.

But for completeness and correctness, move all web utils to webutils.

celery: rename celerypylons to more descriptive celery_app

This module is all about creating a Celery app.

docs: fix broken ref

7982eac0703d removed too much.

repo-scan: recommend running this to reinstall Git hooks after upgrading

It would be too invasive to fully automate this.

repo-scan: log error and continue after failure creating a scanned repo

Continue after for example:
Exception: invalid repo name Foo

repo-scan: overwrite_git_hooks also implies install_git_hooks

(Otherwise, we should probably report an error when ignoring it.)

docs: describe, visualize, and verify internal code structure and layering

Try to describe something that isn't entirely there yet.

deps.py will help track and minimize violations through checks and
visualization in deps.svg .

scripts: handle running with pending deleted files

Don't fail on files in manifest missing in working directory.

diff: handle some escaped characters in Git diffs

There are some odd characters (like \r and \n) that the Kallithea UI doesn't
allow in filenames in repos. Kallithea (through the routes module) will fail to
generate URLs when browsing Files. That is a known limitation with minimal
real-world impact, non-trivial to work around or fix.

There are very few relevant use cases for tracking files with odd filenames. \t
is valid but is hard to render in a meaningful way in the UI. And ASCII
characters like \ and " are not usable on Windows and should just be avoided.

Kallithea would parse Git diffs with odd characers incorrectly or fail, even
before hitting the known limitation. With this change, Kallithea will parse
diffs with odd filenames correctly (and then hit the limitation).

Git will quote odd filenames and escape the odd characters when emitting diffs.
(Mercurial does by design not allow \r and \n , and Mercurial will thus never
have to quote file names in diffs.)

Quotes are already handled (and ignored). With this change, Kallithea will
handle \ unescaping of \\ and \", the usual letters like \r and \n and \t, and
octal numbers like \033 (for ESC) .

Filenames with \ and " will work perfectly (when not on Windows).

Filenames with \t and ESC will work fine, but without helpful display in the
UI.

Filenames with \r and \n will still make the UI fail when trying to generate
URLs.

Thanks to stypr of Flatt Security for raising this.

diff: improved handling of Git diffs with " quoting

Kallithea would intentionally and explicitly fail with an ugly exception when
trying to parse Git diffs with quoted filenames.

Improve this by parsing quotes ... and ignore them, as long as they are
matching. The content inside the quotes might be \-escaped ... but that could
potentially also be the case without quoting. We will fix that later.

Adding some test cases that before would have failed to parse and raised an
exception.

Thanks to stypr of Flatt Security for raising this.

hg: make sure _check_url only accept the protocols accepted by is_valid_repo_uri

Avoid unnecessary flexibility, ambiguity, and complexity.

This is similar to what 3ea3d3a2b3e3 did for Git.

vcs: use urlparse in _check_url

This makes URL checking more standard and more explicit.

vcs: always pass str to _check_url

hg would sometimes pass bytes.

vcs: slight clean up of _check_url

Make static void function static and without return value.

model/user: 'body' parameter to NotificationModel().create() is unused so remove it

The email template for new user registration mails does not use the 'body'
parameter, so don't pass anything.

cleanup: get rid of redundant assignments 'foo = foo'

Redundant assignments were searched with:

grep '^ *\(.*\) = \1$' `hg files`

Some of these cases have resulted from earlier cleanup, like:

foo = safe_unicode(foo)

and later batch removing safe_unicode usage. See e.g. commit
e35373106528b29461b7f5144780cb9ec79f765d .


Note: two false positives occur:

kallithea/tests/models/test_settings.py:39:
setting.app_settings_value = setting.app_settings_value
kallithea/tests/models/test_settings.py:42:
setting.app_settings_value = setting.app_settings_value

which is not a plain assignment but where 'app_settings_value' is a property
method with side effects.

model/comment: drop unused fields in email_kwargs

Keep things as simple as possible.

model: remove unused 'subject' parameter of NotificationModel.create()

The subject of emails is determined with EmailNotificationModel._subj_map,
based on the notification type. The 'subject' parameter passed to
NotificationModel.create is completely unused.

Remove this parameter and update its callers, removing code that is now no
longer used.

model/comment: inline _get_notification_data

This method was only used in one place, from code which was very related.
Inline it to group things together and reduce complexity.

model/comment: extract notification-related code into a separate method

Preparation for grouping with _get_notification_data next, and keeping clear
separation between creating the comment itself, and creating the
notification.

mail: migrate to modern email.message instead of using old compat32 functionality

Before, the HTML part could be detected as pure 7-bit and sent as:
Content-Type: text/html; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Now, it will use:
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

That should work fine too.

mail: fix duplicate "From" headers

Problem introduced in 9a0c41175e66: When iterating the headers dict and setting
"msg[key] = value", it wasn't replacing the header but performing add_header so
we sometimes ended up with two From headers.

It is also a general problem that while the headers dict only can contain each
key once, it can contain entries that only differ in casing and thus will fold
to the same message header, making it possible to end up adding duplicate
headers.

"msg.replace_header(key, value)" is not a simple solution to the problem: it
will raise KeyError if no such previous key exists.

Now, make the problem more clear by explicitly using add_header.

Avoid the duplication problem by deleting the key (no matter which casing)
before invoking add_header. Delete promises that "No exception is raised if the
named field isn’t present in the headers".

front-end: use 'bin' path for node commands instead of '.bin'

license-checker is using relative paths for importing other modueles - that
worked fine when .bin/license-checker was a symlink, but not on filesystems
without symlinks support:

internal/modules/cjs/loader.js:883
throw err;
^

Error: Cannot find module '../lib/index'

middleware: drop appenlight integration

It seems trivial and is currently not hooked up and has not been tested with py3. It is thus better to drop it.

hooks: move the vcs hook entry points and setup code out of lib

Mercurial hooks are running in a process that already has been initialized, so
they invoke the hooks lib directly. Git hooks are binaries and need a lot of
initialization before they can do the same. Move this extra setup code
elsewhere.

Having this high level code in bin is perhaps also not ideal, but it also
doesn't seem that bad: that is where other command line entry points invoke
make_app.

(It seems like it could be adventageous to somehow use "real" bin commands for
hooks ... but for now we use the home-made templates.)

Note: As a side effect of this change, all git hooks *must* be re-installed
when upgrading.

hooks: import whole hooks module

Minimize impact if there should be cycles.

vcs: move ssh handlers out of "backends"

The ssh handlers are much more high-level than the low-level backends ... and
not used as a part of the "backends" concept at all.

git: detect existing symlink hooks before overwriting - only update plain files

If the existing hook is a symlink, the hook is not what we put in place, and we
should stay away - no matter if it points at something that looks like a
Kallithea hook.

git: write Git hook files atomically

Make sure we don't follow symlinks or inherit permissions from previously
installed hook.

git: minor refactor and clean-up of hook installation code

Make "pre-receive" and "post-receive" more explicit and grepable.

scm: simplify InMemoryChangeset handling

Avoid conditional imports.

imports: try to use global imports unless it is a layering violation

To minimize the impact, do imports that violate the layering at runtime instead
of at import time.

model: changes toward import whole modules

*If* there should be circular dependencies, importing 'from' another module
could fail because the module at that time only was partially imported. That
had to be worked around by importing at runtime instead of globally.

Instead, try to always import whole modules. (But we should still try to avoid
cycles.)

lib: consistently import helpers the same way

Make it easier to grep for any remaining potential layering-violating use of
helpers.

lib: move link_to_ref from helpers to utils2

Less reason to import the messy helpers.

lib: move webhelpers2 and friends to webutils

Gives less of the unfortunate use of helpers - especially in low level libs.

lib: move canonical_url & co to webutils

This gives less of the unfortunate use of helpers - especially in model.

lib: consistently use webutils.url instead of h.url

Templates are still the valid use case for accessing through h.

Simplify mocking of url() function.

lib: drop sparsely used h.not_mapped_error

It might be a good idea, but then we should use it much more consistently ...
and it should probably be done differently. Let's keep it simple and be
consistent.

lib: move locale.py to locales.py to avoid shadowing of standard module

"Fix" spurious problem, seen for example as:

$ python kallithea/lib/annotate.py
Traceback (most recent call last):
File ".../lib64/python3.8/site-packages/mercurial/encoding.py", line 107, in <module>
encoding = locale.getpreferredencoding().encode('ascii') or b'ascii'
AttributeError: module 'locale' has no attribute 'getpreferredencoding'

That happened when something in some other module tried to import stdlib locale
... but somehow would pick up the kallithea locale module and things would
fail.

Stay out of that kind of trouble by using a name that doesn't collide.

diffs: remove unused argument enable_comments and class no-comment

enable_comments was only used to set/not-set the 'no-comment' CSS class.
While this class was emitted, no CSS rule nor any JavaScript logic was
actually using it. Last real usage of that class was removed with commit
e87baa8f1c5bd2488aefc23b95c0db3a04bc8431.

Cleanup the code by not emitting 'no-comment' and remove the
'enable_comments' flag.

style: mark failed comment submissions with red panel heading

Make it more obvious to the user that a comment submission failed: mark the
panel of the failed comment as "panel-danger" so the color of the comment panel
heading changes to red.

Previously, only the user and comment text would fade a bit.

lib/diffs: make sure that trailing tabs are indicated

Between the initial submission and final version of commit f79c40759d6f,
changes were made that turn out to be incorrect. The changes assume that the
later match on trailing tabs will 'win' from the plain 'tab' match. However,
Python 're' documentation says:

As the target string is scanned, REs separated by '|' are tried from
left to right. When one pattern completely matches, that branch is
accepted. This means that once A matches, B will not be tested further,
even if it would produce a longer overall match. In other words, the '|'
operator is never greedy.
https://docs.python.org/3.8/library/re.html

As a result, a trailing tab is seen as a plain tab and not highlighted in a
special way.

Unify the tab handling to make it unambiguous how they should be parsed.

The change diff mainly shows re group numbers shifting.

diff: refactor to avoid complexity of passing unnecessary trailing newline around

_escaper tried to handle these optional newlines ... but did not always preserve
them. Keep it simple.

In normal operations, all diff lines end with \n . We only saw un-terminated
lines in a couple of old test cases.

hg: support update with Mercurial 5.6

Fix:
TypeError: update() takes from 1 to 3 positional arguments but 5 were given

diffs: add doctests for _escaper, also showing incorrect behavior

Add some doctests for the _escaper function.
Note: some tests now show incorrect behavior that will be fixed soon.

cli: fix db-create after removal of 'tests' argument in DbManage

Broken by commit 3afc6bff84bb.

[Thomas De Schampheleire: edited commit message]

lib: drop timerproxy

This module has been unused for forever and does no good.