use.py: import re import sys for fn in sys.argv[1:]: with open(fn) as f: s = f.read() s = re.sub(r'''(<script>)('use strict';)\n( *)''', r'''\1\n\3\2\n\3''', s) with open(fn, 'w') as f: f.write(s)
python use.py $(hg loc 'kallithea/templates/**.html')
config: move WSGI middleware apps from lib to config
These middlewares are full WSGI applications - that is not so lib-ish. The middleware is referenced from the application in config - that seems like a good place for them to live.
config: move various py templates to kallithea/templates/py/
For some reason, we had some python templates in kallithea/config . kallithea.config is mainly the TG entry point, and thus a high level controller thing - not a place to store templates.
Instead, use the templates directory and introduce a new py subdirectory.
With git hook templates in a templates directory, there is no need for tmpl in the name.
routing: move config.routing to kallithea.controllers
Routing doesn't belong in config. Having it there caused unfortunate dependencies.
We do routing the old way. If we did it the new way, it would be defined in the root controller. But for now, we just place it next to the root controller.
scm: make sure __get_repo always returns something
Avoid pytype complaint: File ".../kallithea/model/scm.py", line 438, in get_nodes: No attribute 'scm_instance' on None [attribute-error] In Optional[Any]
Avoid invoking __get_repo with None as parameter when creating repos.
ssh: import binascii directly, instead of using it through base64 module
It is unfortunate that the base64 module is leaking its binascii internals in exception types. We started using binascii through the base64 import in 08af13a090e0, but the import is not public, and pytype thus complains.
When browser zoom is used, the font size increases. As a result, the text balloon icon used in the comment bubble increases as well. However, the rectangle in which the icon resides is kept the same size, causing the result to look bad.
Use 'em' units, which are relative to the font size, rather than pixels, for all relevant style properties of the bubble, including size, padding and border-radius. With these settings, the bubble always looks well, regardless of zoom value.
api: stop using 'Optional', 'OAttr'/'OptionalAttr' classes
There does not seem to be a good reason to use the 'Optional' and 'OptionalAttr' classes. It makes the code harder to understand. And worse, the 'default value' specified is not always used, which can thus give false information to users.
The way Optional was used in the API calls is twofold:
1.either by effectively extracting a value, via Optional.extract(param). If 'param' was indeed specified by the user, then this would yield that user-specified value. Otherwise, it would yield the value declared in the parameter declaration, e.g. param=Optional(defaultvalue).
2.or by checking if a variable is an instance of the Optional class. In case a user effectively passed a value, this value will not be of the Optional class. So if a parameter is an object of class Optional, we know the user did not pass a value, and we can apply some default.
In the declaration of the parameter, the specified default value will only be used if the 'extract' method is used, i.e. method 1 above.
A simpler way to address this problem of default values is just with Python default values, using 'None' as magic value if the default will be calculated inside the method.
The docstrings still specify something like: type: Optional(bool) which is humanly readable and does not necessarily refer to a class called 'Optional', so such strings are kept.
Allow adding and removing reviewers of a pull request with the API call 'edit_reviewers', taking a pull request ID and a list of usernames or userids to add and remove. For single-user operation, a string can be used instead of a list.
Note that the 'kallithea-api' tool only accepts strings, so can only perform single-user operations. Use python 'requests', 'curl' or similar instead if you need to operate on multiple users at a time.
extensions: rename 'rcextensions' into 'extensions' but provide compatibility
The 'rc' prefix is legacy. Rename 'rcextensions' into 'extensions', updating all references. Compatibility with the old name will be retained for a while, and removed in a later release. Migrating is as simple as renaming a file.
- use 'try..except' rather than 'if-then', which removes the need for a separate 'path' variable and is more 'pythonic'. - promote log from 'debug' to 'info' and execute it immediately after loading before anything else. - clarify comments related to INDEX_EXTENSIONS
There seems to be no reason that in indirection is needed between the actual hook name and the implementation. Moreover, the implementation names were unnecessary complex.
db: introduce constraint ensuring no duplicate (reviewer, pullrequest) combinations
A reviewer should only be added once to a review.
Previously, this was not ensured by the database itself, although that the controller would try to not add duplicate reviewers. But there was no hard guarantee: e.g. simultaneous adding of the same reviewer to the same review by a review owner and admin, a framework bug that sends the same request twice, ... could still trigger duplicate addition. Additionally, code changes (e.g. a new API) could introduce bugs at the controller level.
Existing production databases were found to contain such duplicate entries. Nevertheless, as the code displaying reviewers in a pull request filtered out duplicates, this never showed in the UI, and never was a 'real' problem.
Add a UniqueConstraint in the database to prevent such entries, with a database migration step that will first find and remove existing duplicates.
model: handle redundant reviewers in add_reviewers
Don't attempt to add reviewers that are already a reviewer for the specified PR (redundant reviewers).
Return the list of added and redundant reviewers, for the controller to handle.
Under normal circumstances, the pullrequest controller already processes the list of reviewers and only calls add_reviewers for new reviewers. But, there could be ways were this checking fails, for example due to a race condition between two simultaneous requests for the same pullrequest, or due to a bug in the web server framework that causes the same request to be handled again.
There are cases where the firstname/lastname known to Kallithea, is not complete, perhaps only an initial. But users that try to add a reviewer, may start typing a full name and expect autocompletion.
For example:
firstname: John lastname: D email: john.doe@example.com
When a review starts typing 'Doe' they will not get any matches if autocompletion is purely based on names.
This situation sometimes arises with Indian developers, where the names known to companies may be abbreviated as initials. For example "C K Deepak" where Deepak is the first name and C K are the initials of the last name.
autocomplete: also query 'firstname lastname' and 'lastname firstname' combinations
The autocomplete functionality for user names, e.g. in pull request reviewer lists, @mentions, etc. would match the input term only on firstname, lastname or username, but not a combination of firstname lastname.
This is a problem when there are many matches on the same firstname or lastname, in particular with Chinese names like 'Wang', 'Cheng', etc. If you know the full name and type it, you would not get any matches.
Instead, adapt the queries to also match on 'firstname lastname' and 'lastname firstname'.
This means that simple matching on only username or only lastname, can be removed.
Ed Wong reported problems with a SSH key that accidentally was copy-pasted with extra newlines. This truncation wasn't detected, so the truncated key was added to authorized_keys where it obviously didn't work for sshd.
The base64 decoding would sometimes catch truncated keys - but not always. We seem to have to look inside the key, parse it according to the RFCs, and verify they contain the right amount of data for the key type.
It is an additional burden to have to parse SSH key internals just to validate them. We could consider using some external method for validation. But the explicit validation introduced here might be more spot-on for our needs.
Also, there is not much essential code in the old sentry support, and it seems like it would be easier to reimplement from scratch. There is thus not much lost by dropping it.
According to documentation, hmac.new is the way to create a HMAC object ... and the first argument is mandaatory and we don't want to name it.
This has no functional change but will address a pytype warning:
File "kallithea/model/user.py", line 304, in get_reset_password_token: Invalid keyword arguments (digestmod, key, msg) to function HMAC.__init__ [wrong-keyword-args]
rcmail: pass smtplib.SMTP.sendmail to_addrs as list
Passing it as a set worked ... but is apparently wrong. The documentation states it has to be a "list of addresses".
Pytype warned:
File "kallithea/lib/rcmail/smtp_mailer.py", line 99, in send: Function SMTP.sendmail was called with the wrong arguments [wrong-arg-types] Expected: (self, from_addr, to_addrs: Union[Sequence[str], str], ...) Actually passed: (self, from_addr, to_addrs: set, ...)
Prepare for computing each kind of permissions separately so they can be skipped in the common case.
This will undo a part of 1ecd6c0e2787 where we took the detour of assigning permissions in __init__ - now we move it back to a LazyProperty, but in a better way.
config: only assign kallitha.CONFIG once and keep it as a plain dict
Mute pytype warnings and make code more readable for developers:
File "kallithea/lib/hooks.py", line 318, in _hook_environment: No attribute 'global_conf' on Dict[nothing, nothing] [attribute-error] File "kallithea/lib/hooks.py", line 318, in _hook_environment: No attribute 'local_conf' on Dict[nothing, nothing] [attribute-error] File "kallithea/bin/kallithea_cli_base.py", line 82, in runtime_wrapper: No attribute 'global_conf' on Dict[nothing, nothing] [attribute-error] File "kallithea/bin/kallithea_cli_base.py", line 82, in runtime_wrapper: No attribute 'local_conf' on Dict[nothing, nothing] [attribute-error]
cli: refactor db_create to avoid app initialization in multiple places
Make sure kallithea.config.application.make_app only is invoked at a high level ... and avoid references to kallithea.CONFIG.global_conf right before kallithea.CONFIG is set to a plain dict again.
Click commands that have both needs_config_file=True and config_file_initialize_app=True will now be called twice - first time before setting kallithea.CONFIG but with the config dict as parameter.
That seems kind of intuitive and will simplify the code and allow cleanup of config handling.
Mute pytype warning:
File "kallithea/bin/kallithea_cli_db.py", line 73, in db_create: No attribute 'global_conf' on Dict[nothing, nothing] [attribute-error] File "kallithea/bin/kallithea_cli_db.py", line 73, in db_create: No attribute 'local_conf' on Dict[nothing, nothing] [attribute-error]
ssh: update authorized_keys after deleting a user with ssh keys
This fixes a minor problem with minimal impact: The authorized_keys entries would fail anyway when the referenced user wasn't found ... and the entries would go away next time authorized_keys for some reason were updated.
No release notes yet, but testing shows it working with a few changes.
A few tests need updating because error messages from remote ssh like our sys.stderr.write('abort: %s\n' % error) like for remote: abort: Access to %r denied will now appear on stderr instead of stdout - a very reasonable bug fix.