lib: establish py3 compatible strategy for string handling: introducing safe_bytes and deprecating safe_str
The meaning of safe_str will change when moving to py3. All use of safe_str is thus tech debt that we have to chop off, mostly by moving to either safe_unicode or safe_bytes ... or dropping because we know what we are doing and rely on the improved type safety in py3.
We will soon move to Python 3 which only will support 5.1 or later.
Remove old hacks and tech debt.
Also avoids future warning: DeprecationWarning: inspect.getargspec() is deprecated since Python 3.0, use inspect.signature() or inspect.getfullargspec()
lib: only maintain one copy of safe_str / safe_unicode
The standalone-ish nature of vcs gets a bit in the way. It already depends on some very generic Kallithea functionality. But for now, avoid code duplication, and let Kallithea use vcs functionality instead of duplicating it.
feeds: replace webhelpers.feedgenerator with simple mako templating for rendering RSS/Atom XML
Most of the complexity in RSS libraries is in dynamically supporting all kinds of attributes. For our use, we have a small static set of attributes we use, and it is simpler to just use mako.
Also, webhelpers is dead, and the alternatives seem quite heavy.
repo: don't just report user name and email in one field - separate things properly
In the repo RSS feed, report author as <author>name@example.com (User Name)</author> instead of using <dc:creator xmlns:dc="http://purl.org/dc/elements/1.1/">User Name <name@example.com></dc:creator>
And in the ATOM feed with name and email separate: <author> <name>User Name</name> <email>name@example.com</email> </author> Instead of <name>User Name <name@example.com></name>
journal: don't include email in author name - avoid double data
In journal RSS feed, report author as: <author>name@example.com (User Name)</author> instead of double email due to: <author>name@example.com (User Name <name@example.com>)</author>
In the journal ATOM feed, report author as: <author> <name>user Name</name> <email>test_admin@example.com</email> </author> instead of using double email due to: <name>name@example.com (User Name <name@example.com>)</name>
app: drop finally handling in BaseVCSController.__call__
Our optional 'wrapper' middleware is logging response times more correctly after b42ee1bdf082 - there is no point in logging the timing of the __call__ in the main app.
Similarly, the session was removed too early. But TurboGears is already picking up our db session and using DBSessionRemoverMiddleware to .remove() it ... and at the right time. So just stop trying.
utils: drop make_ui parameter clear_session - this utility function should never mess with sessions
clear_session seems to have been based on the idea that utility functions that happened to create a session also should clean up. But instead, we now consistently take care of session removal at a high level.
app: let app instance creation remove its SA Session when done
The app instance is usually created early or on demand before serving the first request, and the app instance might sit idle for a while before receiving the first request. The app creation will touch the database, and we have to consider the lifetime of the database session it is using. The cbf524e4c1a3 commit message gives some reasons we should remove the session when done.
But instead of handle it inside utility functions, we would rather handle sessions at a high level as here.
The caching condition has been False since ffd45b185016 . That makes sense - caching as implemented would leak the repo list of other users without considering permissions. The repo list is however only loaded on demand, and caching is not that important.
routing: fix files_annotate_home annotate value to be compatible with Routes >= 2
The routing entry for files_annotate_home had annotate=True. That primarily served to let the controller files.index differentiate files_annotate_home from files_home and files_home_nopath . Anything true can work.
test_files.py is creating files_annotate_home URLs in an odd way: Instead of explicitly specifying it is a files_annotate_home URL, it passes expected controller parameters and expects routing to find a URL that would provide these parameters. It thus also has to specify the otherwise "invisible" annotate value. For Routes < 2, True works just fine. For Routes >= 2, it seems to expect values that actually can be encoded in URLs.
routing: drop default f_path for changelog_file_home
There is no point in creating a changelog_file_home URL without specifying f_path. The default value of None is thus never any good. And especially, it is unclear what a value of None really means.
When attempting to use ed25519 SSH keys, parse_pub_key() failed with: SshKeyParseError: Incorrect SSH key - base64 part is not 'ssh-ed25519' as claimed but 'ssh-ed25519'
The problem was the hardcoding of the string length of the key type -- 7 or '\x07' -- which fits ssh-rsa and ssh-dss but not ssh-ed25519.
scripts/make-release: install ldap and pam to fix isort instabilities
isort (triggered by scripts/whitespacecleanup.sh) needs to know which modules are local and which are not, to separate them with a newline. If a module cannot be found, it will be treated as local, apparently.
When ldap is not installed in the current environment, a difference was created by isort in kallithea/bin/ldap_sync.py.
Commit 04dee6fdfdff fixed an apparent problem detected by isort, but as Mads Kiilerich pointed out, it was caused by an incomplete virtualenv that did not include 'python-ldap'. As a result, isort would not consider 'ldap' as a standard module and treated it as a local module.
A subsequent commit will fix the 'make-release' script to install all expected dependencies.
admin: fix 'Settings > Visual' form validation after commit 574218777086
Commit 574218777086 introduced a setting for 'SSH Clone URL' in 'Admin > Settings > Visual' and placed it under a check 'if c.ssh_enabled', which means that the corresponding form field is not present when SSH is not enabled. In this case, when trying to save the form (changing any or none setting), form validation reports an error 'Missing value' without much detail. At the top of the HTML document, even before the opening HTML tag, we can see:
Got warnings like: kallithea/tests/other/test_vcs_operations.py::TestVCSOperations::test_yada_yada .../site-packages/webob/acceptparse.py:649: DeprecationWarning: The behavior of AcceptValidHeader.__contains__ is currently being maintained for backward compatibility, but it will change in the future to better conform to the RFC. DeprecationWarning,
There *must* be a better alternative to pygrack ... for now, let's keep it alive.
page: drop most paginate customizations - the bare implementation seems good enough
The main difference from this seems to be that redundant controls (such as "previous" on first page) no longer will be shown.
default_link_tag is based on the base class implementation with minimal changes, wrapping links in <li> as before, and with different handling of current_page to emit markup as before.
page: minimal change to move from webhelpers.paginate to paginate
webhelpers is dead and doesn't work with py3. paginate is not very actively maintained, but it is the natural successor to webhelpers.paginate, it seems stable, and it works with py3.
This is a minimal change that seems to work. It preserves existing tech debt ... and adds a little bit more. It will be cleaned up next.
webhelpers.paginate had built-in SqlAlchemy support - now we have to handle it explicitly.
page: replace RepoPage with Page given the reverse collection
That seems to be all RepoPage did ...
We must still take care to make sure the collection works correctly, even when filtered so indices might be higher than repo length. vcs module takes care of that by internally creating a list of hashes (which it can reverse), while the Changeset instances are only created on demand. We can save some resources by not retrieving the whole list of Changesets just to reverse it so we can use a few entries.
Show "There are no changesets yet" instead of relying on pager to silently eat EmptyRepositoryError. Use .get_changesets() instead of using c.db_repo_scm_instance directly.
vcs: fix get_changesets to .reverse() list of revision hashes in place instead of using reversed()
This is slightly more performant, and will also make CollectionGenerator work with reverse=True. Before, CollectionGenerator was used on get_changesets, but never with reverse option. Trying to use reverse, it would fail when applying len():
TypeError: object of type 'listreverseiterator' has no len()
db: migration step after 95c01895c006 failed to add usk_public_key_idx in alembic step b74907136bc1
Use meta reflection to check if the indices exists before running the upgrade step. The upgrade step only has to be run if it is an old database - not if it has been created after the schema changes were introduced.
db: introduce migration step after 93834966ae01 dropped non-nullable inherit_default_permissions
The database migration step was lazily and naively skipped ... but that turns out to be a problem when new users are added. In the database, the original column 'inherit_default_permissions' was marked as non-nullable without default value. In the Kallithea code after commit 93834966ae01, the column 'inherit_default_permissions' was no longer known, and thus not given a value when new users are added. As a result, the database complained:
IntegrityError: (psycopg2.errors.NotNullViolation) null value in column "inherit_default_permissions" violates not-null constraint
Fix that now by adding an appropriate db migration step to actually remove the columns.
Use meta reflection to check if columns exist before running the upgrade step. The upgrade step only has to be run if it is an old database - not if it has been created after the schema changes were introduced.
For the downgrade step, make sure to set a default value for non-nullable columns.
Since a jQuery upgrade in commit c225c37c069d, 2-way diff was broken: the height was not correct, and sometimes the source code was shown in gray boxes.
Analysis showed that in the invocation of mergely (templates/files/diff_2way.html), '$("#footer").height()' is undefined, in turn caused by the absence of an HTML element with id 'footer'.
The 'id' property on the footer was removed in commit 61c99cdbbfff, retaining only the 'class="footer"'.
Fix the problem by using the class-based selector to get the footer height.
As the footer height will now be an actual value instead of '0' originally, we can update the calculation without 'magic' values like '36' which was actually a reference to the footer size. When we initialize mergely, the page only contains the header and footer. All window space below the footer can be assigned to the compare panes. The height specified to mergely is thus the total window height minus the header height (the top position of the footer) and the footer height.
search: avoid crash when making (odd) search for '*'
Crashed in whoosh ListMatcher.supports() on def supports(self, astype): return self._format.supports(astype) with AttributeError: 'NoneType' object has no attribute 'supports' on for example http://localhost:5000/_admin/search?q=*&type=content .
There doesn't seem to be a good way to detect if _format has been provided.
helpers: replace webhelpers.flash with own implementation
webhelpers is dead.
One small function implements pretty much the same functionality, using the same session key so tests still pass, but also very simple and without external dependencies.
It could be implemented with a class and different methods for adding, getting and clearing. But internally, it would probably have pretty much the same helper function has here. So let's just avoid the unnecessary complexity and keep it simple.