This makes linting rules in zulint more general. Make necessary
changes in tools/lint and tools/custom_check.py to run with the new
RuleList class.
Modify tests for `RuleList` class. Tests only include minor changes to
test with the new class.
Previously, it didn't properly update the stamp files that determine
our caching behavior, so if one ran test-backend afterwards, nothing
would happen.
A secondary issue that this commit does not fix is that provision will
end up rerunning the whole thing.
This fixes an issue where one could end up with a `(` in the markdown
syntax for a link after copy-pasting this, which doesn't work in
markdown.
Fixes#12579.
A function was written in `test_fixtures.py` to drop a test database
template if the corresponding database id doesn't belong to a file.
Alongside this fact, every file that is written is removed after 60
minutes. Meaning any potential database template can never exist
longer than one hour.
This follow-up work was added to deal with the potential race
conditions when running `test-backend`. Ensuring that all templates
are properly dealt with.
Essentially rewritten by tabbott for cleanliness.
Fixes the remainder of #12426.
When running the `./tools/cache-zulip-git-version` script on Travis, the script
fails because Travis gets a shallow clone of the repository, and not a full
clone. This commit changes the script to fail gracefully, if we are unable to
get the version information using `git describe`.
When the command fails, it still writes an empty `zulip-git-version` and that
has not been changed to keep creation of the release tarball simple, and
avoiding a check for whether the file has any content. The code that sets
`ZULIP_VERSION` checks whether the contents of the `zulip-git-version` file are
empty, before setting `ZULIP_VERSION`. So, the version should never be set to an
empty string.
More modern Linux versions like Bionic will block this, and what we
actually want to do is just run the code in our <<EOF block via bash,
so we should do that explicitly.
The integrations page had css in both `landing-page.scss` and
`portico.scss`. With this commit, the styles are mostly unified into
a single separate file.
This moves all the stylesheets like stats, billing etc. to another
directory called `static/styles/portico/`, matching the directory
structure of our JavaScript.
We use `git describe --tags` to get information about the number of commit since
the last major version, and the sha of the current HEAD. This is added to the
ZULIP_VERSION when a deploy is done from `git`.
Modified heavily by punchagan to:
* to use git describe instead of `git log` and `wc`
* use a separate script to run the git describe command
* write the file with version info to var/ and remove it from the repo
Fixes#4685.
Profiling shows that using cache-loader saves ~6-7 seconds of time take
by webpack-dev-server on subsequent runs. The overhaul this adds when
nothing is cached (when running first time) is around 1-2 seconds. We don't
use cache loader for ts-loader since webpack docs says it will slow it down
and file-loader since it just copies files over and caching it would just
was disk space.
This is the second merge of this commit. It fixes the issue with the
previous one by placingn cache-loader after mini-css-loader because it
just extracts css and caching that will make file-loader not run which
in turn makes developement enviorment break.
Profiling shows that using cache-loader saves ~6-7 seconds of time take
by webpack-dev-server on subsequent runs. The overhaul this adds when
nothing is cached (when running first time) is around 1-2 seconds. We don't
use cache loader for ts-loader since webpack docs says it will slow it down
and file-loader since it just copies files over and caching it would just
was disk space.
Profiling data:
-------- Master ---------
~/zulip (master) $ tools/webpack --watch | ts -s '%.S' # master
03.995825 ℹ ｢wds｣: Project is running at http://127.0.0.1:9994/
03.996161 ℹ ｢wds｣: webpack output is served from /webpack/
03.996289 ℹ ｢wds｣: Content not from webpack is served from ...
19.284477 ℹ ｢wdm｣:
19.285371 ℹ ｢wdm｣: Compiled successfully.
-------- cache-loader ---------
~/zulip (cache-loader)$ tools/webpack --watch | ts -s '%.S'
04.107913 ℹ ｢wds｣: Project is running at http://127.0.0.1:9994/
04.108646 ℹ ｢wds｣: webpack output is served from /webpack/
04.109068 ℹ ｢wds｣: Content not from webpack is served from ...
12.633782 ℹ ｢wdm｣:
12.634083 ℹ ｢wdm｣: Compiled successfully.
Before we used to defined our own type Loader which was
partly incorrect because the use property can only
be string which is incorrect. We use the RuleSetRule type
provided by webpack instead.
Real systemd requires this. docker-systemctl-replacement currently
doesn’t but maybe it will later.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This exchanges a race condition where webpack-dev-server might not be
stopped on a poorly timed KeyboardInterrupt for a less bad race
condition where we might get an UnboundLocalError.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This should make the run-dev.py user experience a lot nicer when
switching branches away from a branch that is at least as new as this
commit, since we won't need to manually restart run-dev.py to restart
webpack.
Fixes#9042.
This doesn’t seem to add any noise in the normal case, but if anything
shows up here we might want to see it.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
* Remove the custom has_error logic in favor of checking whether any
errors were logged, which gives us a much better chance at catching
unanticipated exceptions.
* Use our error_callback for the initial requests of start_urls too.
* Clean up mypy types.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The jedi package exec()s some code in the context of the fake module
blub, causing errors when generating the coverage report. See
https://github.com/davidhalter/jedi/issues/1122.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The test-backend parallel test runner system doesn't actually use the
zulip_test database; instead, it creates its own databases off the
zulip_test_template database.
We were accidentally running `tools/generate_fixtures` even when there
are no changes, because this function is shared with the
tools/lib/test_server.py codebase, which needs us to do the work of
creating a test database for it off the zulip_test_template database.
Fixing this saves about 1.5s / 4s of the runtime of a single test.
This restores man pages and other documentation that have been
stripped from the default Ubuntu cloud image and installs
ubuntu-minimal and ubuntu-standard.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
perfect-scrollbar replaces both the appearance and the behavior of the
scrollbar, and its emulated behavior will never feel native on most
platforms. SimpleBar customizes the appearance while preserving the
native behavior.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Upgrades to the stripe library can sometimes break semantics for our
billing system, and so we should make sure to use our documented
testing process before doing them.
Using sys.exit(1) in a management command makes it impossible
to unit test the code in question. The correct approach to
do the same thing in Django management commands is to raise
CommandError.
This commit adds a new developer tool: The "integrations dev panel"
which will serve as a replacement for the send_webhook_fixture_message
management command as a way to test integrations with much greater ease.
This lets us handle directly in our tooling the user experience that
we document for exporting a realm with member consent (before, it
required unpleasant manual work).
This commit migrates the Subscription's notification fields from a
BooleanField to a NullBooleanField where a value of None means to
inherit the value from user's profile.
Also includes a migrations to set the corresponding settings to None
if they match the user profile's values. This migration helps us in
getting rid of the weird "Apply to all" widget that we offered on
subscription settings page.
The mobile apps can't handle None appearing as the stream-level
notification settings, so for backwards-compatibility we arrange to
only send True/False to the mobile apps by applying those defaults
server-side. We introduce a notification_settings_null value within a
client_capabilities structure that newer versions of the mobile apps
can use to request the new model.
This mobile compatibility code is pretty effectively tested by the
existing test_events tests for the subscriptions subsystem.
It was discovered that the '.eslintcache' file was causing eslint to
throw a TypeError after a recent update/addition to the dependencies.
It makes sense to remove this file as part of the provisioning process
to avoid such exceptions.
This commit removes `tools/check-urls`. It was added as
a useful tool in preparation for the Django 1.10 migration.
Since we completed that migration, it is no longer needed.
Fixes#12180.
The number of processes to run the backend tests is currently a
hardcoded value, this commit transistions the default to be based on the
number of logical CPUs available.
This commit adds `stream_ui_updates.js` module. This module
will includes functions which will update different ui elements
(i.e. subscription button, subscriber count).
The github-services model for how GitHub would send requests to this
legacy integration is no longer available since earlier in 2019.
Removing this integration also allows us to finally remove
authenticated_api_view, the legacy authentication model from 2013 that
had been used for this integration (and other features long since
upgraded).
A few functions that were used by the Beanstalk webhook are moved into
that webhook's implementation directly.
This is really a job for an AST parser rather than a pile of regexes;
among other issues, these will still miss violations that span
multiple lines. But, you know, I tried.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Also use psql -e (--echo-queries) in scripts that use ‘set -x’, so
errors can be traced to a specific query from the output.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
`tools/run-dev.py` already backgrounds `tools/webpack` (and deals with
cleaning it up on exit), so there’s no need for `tools/webpack` to
also background the actual `webpack` process. But when running
`tools/webpack` by itself, it’s annoying to clean up the backgrounded
process manually.
Run `webpack` in the foreground, using `os.execvp` so we don’t waste
memory on an intermediate wrapper process.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Webpack applies special logic to relative paths provided in
`resolve.modules`, and this logic is expected to be used for
`node_modules`. One case where this is important is when
`node_modules/foo` wants to import a different version of package
`bar` than the one at `node_modules/bar`, and so yarn gives it its own
copy at `node_modules/foo/node_modules/bar`.
It would probably be better to avoid screwing with `resolve.modules`
at all, but this at least brings us one step closer to the default of
just `["node_modules"]`.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
activate_this.py has always documented that it should be exec()ed with
locals = globals, and in virtualenv 16.0.0 it raises a NameError
otherwise.
As a simplified demonstration of the weird things that can go wrong
when locals ≠ globals:
>>> exec('a = 1; print([a])', {}, {})
[1]
>>> exec('a = 1; print([a for b in [1]])', {}, {})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<string>", line 1, in <module>
File "<string>", line 1, in <listcomp>
NameError: name 'a' is not defined
>>> exec('a = 1; print([a for b in [1]])', {})
[1]
Top-level assignments go into locals, but from inside a new scope like
a list comprehension, they’re read out of globals, which doesn’t work.
Fixes#12030.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The dependency visualizer currently only supports JavaScript files,
such as in the `get_js_edges` function, where only the ".js" extension
is supported. Update the visualizer to support ".ts" files as well and
to output modules without their extensions.
Currently, the `test-js-with-node` tests append ".js" to filenames
without an extension. Since Typescript is now also supported, it can
produce results such as "dict.ts.js". To remedy this, check for ".ts"
files as well.
Spider raises exceptions when errors like FileNotFound
are detected. However, these did not set error state
before exiting causing spider to fail silently.
This patch sets the status causing exceptions to exit with
non-zero exit status.
All the inline javascript code present in email_log.html(which is
rendered when the user visits "/emails" in development mode) is
transferred to a new file: email_log.js in portico/ directory.
Fixes#11608.
As of #367, `tools/run-dev-queue-processors` has evolved into nothing
more than an unnecessarily elaborate wrapper around `manage.py
process_queue --all`. Remove it (mostly to make it marginally easier
to Tab-complete `tools/run-dev.py`, if I’m being honest).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The delete operator could throw a TypeError when attempting to
remove a non-configurable property, which is rare in practice since
they can only be created using `Object.defineProperty()` and
`Object.freeze()`. We also never uses the output of `del()` anyway.
A new javascript file "dev-login.js" is created in static/js/portico/
and the inline javascipt code present in dev_login.html is transferred
to that file. An empty div element is added in dev_login.html with
unique data-page-id attribute to make it more easy to find in which
page we are, while working with the javascript code.
This generalizes the provision logic for deciding whether to build our
tsearch_extras and pgroonga search extensions from source to support
Ubuntu cosmic as well (and evenutally, other future platforms).
It appears that this code did the right thing despite being written
wrong, probably due to whatever `manage.py collectstatic` does in its
argument parsing. But in any case, we should make the code read how
it's intended.
Accomplished by adding a function to clear the status message with
an empty string. The html is then updated to reflect changes without a
refresh.
Currently, it's a small hassle to clear a status message. This option
makes things a bit easier.
Fixes#11630.
This optimizes test-backend by skipping webhook
tests when run in default mode.
Tweaked by tabbott to extend the documentation and update the CI
commands.
This is mostly adding markup, calling some convenient
functions in buddy_data.js, and adjusting CSS.
To make the circles update dynamically, I mostly
orchestrate this though activity.js for now. It's
possible we'll want to adjust that eventually to
happen through something like a `presence_events`
dispatcher, but that's essentially what
a good part of `activity.js` does now.
Commit 7d12e2019d (#10994) broke fresh
provisions by importing zproject.settings before we were done
modifying settings. Fix it by moving the generate_secrets invocation
to the earliest reasonable place.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit does the following three things:
1. Update stream model to accomodate rendered description.
2. Render and save the stream rendered description on update.
3. Render and save stream descriptions on creation.
Further, the stream's rendered description is also sent whenever the
stream's description is being sent.
This is preparatory work for eliminating the use of the
non-authoritative marked.js markdown parser for stream descriptions.
Instead, manually activate it in the one place where this
functionality was used (tools/lib/provision.py). This way we avoid
trying to activate the Python 2 thumbor virtualenv from Python 3.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This seems to be a common enough pitfall to justify
a bit of extra handling. Example output:
$ ./tools/run-dev.py
Clearing memcached ...
Flushing memcached...
OK
Starting Zulip services on ports: web proxy: ...
Note: only port 9991 is exposed to the host in a Vagrant environment.
ERROR: You probably have another server running!!!
Traceback (most recent call last):
File "./tools/run-dev.py", line 421, in <module>
app.listen(proxy_port, address=options.interface)
File "/srv/zulip-py3-venv/lib/python3.5/...
server.listen(port, address)
File "/srv/zulip-py3-venv/lib/python3.5/...
sockets = bind_sockets(port, address=address)
File "/srv/zulip-py3-venv/lib/python3.5/...
sock.bind(sockaddr)
OSError: [Errno 98] Address already in use
Terminated
NOTE: If you revert this commit, you want to revert
the immediately prior commit as well. The history
is that Ishan made some improvements to the widget,
but there were some minor bugs. I decided not
to squash the commits together so that the git
history is clear who did what. (In particular, I
want questions about the JS code to come to me if
somebody does `git blame`.)
Anyway...
This is a fairly significant rewrite of the polling
widget, where I clean up the overall structure of
the code (including things from before the prior
fix) and try to polish the prior commit a bit as
well.
There are a few new features:
* We tell "other" users to wait for the poll
to start (if there's no question yet).
* We tip the author to say "/poll foo" (as
needed).
* We add edit controls for the question.
* We don't allow new choices until there's
a question.
Apparently, the missed-message templates have a slightly different
structure from our other email templates, which triggered a latent,
subtle bug in inline-email-css's effort to remove duplicate <html>
blocks from emails that had been generated by premailer. Fix this
bug, and add appropriate assertions to prevent similar issues in the
future.
Fixes#11249.
We still create a Python 2 virtualenv for thumbor but that’s
separate (/srv/zulip-thumbor-venv from
scripts/lib/create-thumbor-venv).
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
As part of this change, we port into the .messages class the work in
4e8e7348da to change overflow-y to auto,
not scroll (skipping that would result in a regression).
Otherwise this causes an error
```
AttributeError: type object 'Callable' has no attribute '_abc_registry'
```
on 3.7. While the error is specific to 3.7, it is safer to uninstall
typing for all the versions that don't require a pip-provided typing
library.
We instead get the specific fields from message
that we use. This is particularly helpful
for subject -> topic migration; we no longer
have to account for "subject" fields in
client-side templates.
The crawler used to be called directly for checking external links.
Now the scrapy command calls the crawl_with_status wrapper.
Crawl_with_status has been modified to pass the external parameter in
the previous commit, so we can now use this simpler approach.
This is a major rewrite of the billing system. It moves subscription
information off of stripe Subscriptions and into a local CustomerPlan
table.
To keep this manageable, it leaves several things unimplemented
(downgrading, etc), and a variety of other TODOs in the code. There are also
some known regressions, e.g. error-handling on /upgrade is broken.
/bin/sh and /usr/bin/env are the only two binaries that NixOS provides
at a fixed path (outside a buildFHSUserEnv sandbox).
This discussion was split from #11004.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This is a common bug that users might be tempated to introduce.
And also fix two instances of this bug that were present in our
codebase, including an important one in our upgrade code path.
To support this, we add a pass_targets option to the main linter
library, because with current mypy, it's generally counterproductive
to pass the list of files in (can produce spurious errors; isn't
faster).
The testing section is more appropriate, since it's fundamentally part
of our CI system.
While we're at it, fix the fact that we were linking to GitHub, not
ReadTheDocs, in the run-mypy output.
This changes a few things:
* Deplicates deps_to_install logic.
* Has a retry flag, under which we can guard the apt retry print statements.
* Makes the install_system_deps flow more parallel.
The fixture changes are because self.upgrade formerly used to cause a page load
of /billing, which in turn calls Customer.retrieve.
If we ran the full test suite with GENERATE_STRIPE_FIXTURES=True, we would
likely see several more Customer.retrieve.N.json's being deleted. But
keeping them there for now to keep the diff small.
This commit works by vendoring the couple functions we still use from
puppetlabs stdlib (join and range), but removing the rest of the
puppetlabs codebase, and of course cleaning up our linter rules in the
process.
Fixes#7423.
This optimizes tools/provision by not running
`tools/update-authors-json --use-fixture` unless either the script
itself or its fixtures file (zerver/tests/fixtures/authors.json) was
changed.
Fixes#10991.
This release is from 2018-08-22, a little over 100 days ago.
It was the first release with the important fix so that when the
server advises it to stop displaying a notification because the user
has read the message (as the SEND_REMOVE_PUSH_NOTIFICATIONS server
setting enables), the app doesn't instead replace the notification
with a broken one reading "null". We have that setting running now
on chat.zulip.org, and intend to roll it out more broadly soon.
The `# take 0` thing is a slightly absurd workaround for the fact
that our funky out-of-line way of marking lines to ignore doesn't
work right if there are multiple such lines in a given file that
are equal modulo leading and trailing whitespace.
This option causes test-documentation to only verify internal links
that we control and are important to be correct. This prevents
test-documentation from flaking in CI due to issues with the dozens of
third-party blocks that we link to from various parts of our
documentation.
Tweaked by tabbott for comment clarity, and to also include
github.com/zulip links.
Fixes#10942.
The goal here was to enforce 100% coverage on
parse_narrow, but the code has an unreachable line
and is overly tolerant of bogus urls. This will
be fixed in the next commit.
This fixes an actual user-facing issue in our mobile push
notifications documentation (where we were incorrectly failing to
quote the argument to `./manage.py register_server` making it not
work), as well as preventing future similar issues from occurring
again via a linter rule.
We now let color_data keep its own state for
unused_colors, so that we longer have to pass in
a large list of unused_colors every time we want
to assign a new stream color.
This mostly matters at startup, where we might
be cycling through 5000 streams. We claim all
the unused colors up front.
Each operation now has an upper bound of expensiveness,
where the worst case scenario is basically popping
off the first element of a list of <= 24 colors.
The algorithm is now deterministic, too, to make
it easier to test. It's unclear whether random color
assignment ever had much benefit, and it made unit
testing the algorithm difficult. Now we have 100%
line coverage.
Fixes part of #10902.
We've had a few unpleasant bugs with real documentation links being
broken, so we're going to make this more aggressive for now.
I think we instead want a more subtle option for suppressing failures
in some places but not others.
This library was absolutely essential as part of our Python 2->3
migration process, but all of its calls should be either no-ops or
encode/decode operations.
Note also that the library has been wrong since the incorrect
refactoring in 1f9244e060.
Fixes#10807.
This seems like kind of a silly function to extract
to topic.py, but it will theoretically help us sweep
"subject" if we change the DB.
It had test coverage.
Even though you'd think these regexes would be
cached, compiling the regex outside of looping
through lines makes a difference.
My timings are 8.4s -> 6.0s. (You need to hack
on the linter to isolate the custom checks.)
We (lexically) remove "subject" from the conversion code. The
`build_message` helper calls `set_topic_name` under the hood,
so things still have "subject" in the JSON.
There was good code coverage on `build_message`.
Previously, a string ending in "... 😄" was reported as an
error and the linter complained that there should be a space
after the last ':'. This commit changes the pattern so that the
linter only checks for colons that are preceded by an opening
double-quote (").
We now prevent adding "subject" to any code in
zerver/lib, unless you specifically exempt it.
The new set called `FILES_WITH_LEGACY_SUJECT`
is also has comments that give a roadmap of what
to fix.
In tools/pre-commit line 18:
if [ -z "$VIRTUAL_ENV" ] && `which vagrant > /dev/null` && [ -e .vagrant ]; then
^-- SC2092: Remove backticks to avoid executing output.
^-- SC2006: Use $(..) instead of legacy `..`.
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.
In tools/pre-commit line 23:
./tools/lint --no-gitlint --force $changed_files || true
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/setup/install-aws-server line 25:
zulip_root=${ZULIP_ROOT:-$HOME/zulip}
^-- SC2034: zulip_root appears unused. Verify use (or export if used externally).
In tools/setup/install-aws-server line 40:
if [ -n "$zulip_confdir" ]; then
^-- SC2154: zulip_confdir is referenced but not assigned.
In tools/setup/install-aws-server line 55:
VIRTUALENV_NEEDED=$(if $(echo "$type" | grep -q app_frontend); then echo -n yes; else echo -n no; fi)
^-- SC2091: Remove surrounding $() to avoid executing output.
In tools/setup/install-aws-server line 60:
SSH_OPTS=(-o HostKeyAlgorithms=ssh-rsa)
^-- SC2191: The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it.
In tools/setup/install-aws-server line 69:
ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lroot <<EOF
^-- SC2087: Quote 'EOF' to make here document expansions happen on the server side rather than on the client.
In tools/setup/install-aws-server line 86:
ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lroot <<EOF
^-- SC2087: Quote 'EOF' to make here document expansions happen on the server side rather than on the client.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/setup/postgres-init-dev-db line 10:
ROOT_POSTGRES="sudo -i -u "$DEFAULT_USER" psql"
^-- SC2027: The surrounding quotes actually unquote this. Remove or escape them.
In tools/setup/postgres-init-dev-db line 46:
echo 'ERROR: Try `sudo service postgresql start`?'
^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
In tools/setup/postgres-init-dev-db line 64:
PGPASS_ESCAPED_PREFIX="*:\*:\*:$USERNAME:"
^-- SC1117: Backslash is literal in "\*". Prefer explicit escaping: "\\*".
^-- SC1117: Backslash is literal in "\*". Prefer explicit escaping: "\\*".
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/django-template-graph line 10:
for t in $(find -name '*.html' -printf '%P\n'); do
^-- SC2044: For loops over find output are fragile. Use find -exec or a while read loop.
^-- SC2185: Some finds don't have a default path. Specify '.' explicitly.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/deploy-branch line 17:
[ $? -ne 0 ] && error_out "Unknown branch: $branch"
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
In tools/deploy-branch line 23:
if [ $? -eq 0 ]; then
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
In tools/deploy-branch line 35:
[ $? -ne 0 ] && error_out "Rebase onto origin/master failed"
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
In tools/deploy-branch line 39:
[ $? -ne 0 ] && error_out "Push of master to origin/master failed"
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/commit-msg line 9:
if [ $(grep '^[^#]' .git/COMMIT_EDITMSG --count) -ne 0 ]; then
^-- SC2046: Quote this to prevent word splitting.
In tools/commit-msg line 10:
lint_cmd="cd ~/zulip && cat \"$1\" | python -m gitlint.cli"
^-- SC2089: Quotes/backslashes will be treated literally. Use an array.
In tools/commit-msg line 11:
if [ -z "$VIRTUAL_ENV" ] && `which vagrant > /dev/null` && [ -e .vagrant ]; then
^-- SC2092: Remove backticks to avoid executing output.
^-- SC2006: Use $(..) instead of legacy `..`.
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.
In tools/commit-msg line 14:
$lint_cmd
^-- SC2090: Quotes/backslashes in this variable will not be respected.
In tools/commit-msg line 17:
if [ $? -ne 0 ]; then
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/clean-branches line 33:
echo -n "Deleting local branch $(echo "$ref" | sed 's!^refs/heads/!!')"
^-- SC2001: See if you can use ${variable//search/replace} instead.
In tools/clean-branches line 41:
echo -n "Deleting local branch $(echo "$ref" | sed 's!^refs/heads/!!')"
^-- SC2001: See if you can use ${variable//search/replace} instead.
In tools/clean-branches line 49:
remote_name="$(echo "$ref" | sed 's!^refs/remotes/origin/!!')"
^-- SC2001: See if you can use ${variable//search/replace} instead.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/build-release-tarball line 50:
for i in `cat "$TMPDIR/$prefix/tools/release-tarball-exclude.txt"`; do
^-- SC2013: To read lines rather than words, pipe/redirect to a 'while read' loop.
^-- SC2006: Use $(..) instead of legacy `..`.
In tools/build-release-tarball line 51:
rm -r --interactive=never "$TMPDIR/$prefix/$i";
^-- SC2115: Use "${var:?}" to ensure this never expands to / .
In tools/build-release-tarball line 97:
echo; echo -ne "\033[33mRunning update-prod-static failed. "
^-- SC1117: Backslash is literal in "\0". Prefer explicit escaping: "\\0".
In tools/build-release-tarball line 98:
echo -e "Check $TMPDIR/update-prod-static.log for more information.\033[0m"
^-- SC1117: Backslash is literal in "\0". Prefer explicit escaping: "\\0".
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/build-docs line 3:
cd "$(dirname "$0")"/../docs
^-- SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit adds a test for the payload that is generated when
a Task is moved from one user story to another on Taiga's Sprint
Taskboard UI.
This commit also gets up this webhook's test coverage up to 100%.
We drop support for usage of `icon-vector` as base class when
including icons from font awesome icons package.
Now on, only icons as specified in font awesome v4.7.0 can be used
in the code base.
This module makes it really easy to create are-you-sure
dialogs for dangerous operations.
Basically it's one function with five parameters. You
give three chunks of HTML, a callback function, and
a parent container.
The first use of this will be in settings_user_groups,
coming up in a couple commits.
IFTTT allows custom templating for their payloads, so the onus is
on the user to ensure that their custom templates conform to the
expectations outlined in our IFTTT webhook docs. For that reason,
these payloads weren't generated, but were manually edited.
After discovering a couple of bugs, I decided to thoroughly test
and rewrite this integration from scratch. The older code wasn't
generating coherent messages.
This also commit gets this integration up to 100% test coverage.
Test coverage was improved by removing an unused function and
removing some code (written by me) that was actually handling
Test Hook event types incorrectly.
It was a painful amount of work to generate the actual payload.
Since the only difference was a small build URL, I manually
edited the payload and used that for testing.
This commit gets our GitHub webhook up to 100% test coverage.
Note that Freshdesk allows custom templating for outgoing payloads
in their webhook UI. Therefore, the payloads added in this commit
did not have to be official payloads from Freshdesk.
The lack of coverage was due to:
* An unused function that was never used anywhere.
* get_commit_status_changed_body was using a regex where it didn't
really need to use one. And there was an if statement that
assumed that the payload might NOT contain the URL to the commit.
However, I checked the payload and there shouldn't be any instances
where a commit event is generated but there is no URL to the commit.
* get_push_tag_body had an `else` condition that really can't happen
in any payload. I verified this by checking the BitBucket webhook
docs.
We shouldn't just ignore exceptions when encoding the incoming
auth credentials. Even if the incoming credentials are properly
encoded, it is better to know when that is the case or if
something else fails.
Apparently, Travis removed the Heroku bundle of packages from their
servers, which made the build start failing when trying to configure
apt to hold their versions (sigh). This commit removes the
problematic packages.
The companion tool `tools/reset-to-pull-request` has a handy feature
to maintain a local ref tracking the PR: e.g., pr/1234 for PR 1234.
If this were a normal remote-tracking branch maintained by `git fetch`,
it'd get updated on `git push`. Do the same thing here.
This helps keep a view like `gitk --all @` a bit tidier, by causing
merged PRs to stop pointing at side branches of the main history.
Before this change, the way we loaded
webpack for various tools was brittle.
First, I addressed test-api and test-help-documentation.
These tools used to be unable to run standalone on a
clean provision, because they were (indirectly)
calling tools/webpack without the `--test` option.
The problem was a bit obscure, since running things
like `./tools/test-backend` or `./tools/test-all` in
your workflow would create `./var/webpack-stats-test.json`
for the broken tools (and then they would work).
The tools themselves weren't broken; they were the
only relying on the common `test_server_running` helper.
And even that helper wasn't broken; it was just that
`run-dev.py` wasn't respecting the `--test` option.
So I made it so that `./tools/run-dev` passes in `--test` to
`./tools/webpack`.
To confuse matters even more, for some reason Casper
uses `./webpack-stats-production.json` via various
hacks for its webpack configuration, so when I fixed
the other tests, it broke Casper.
Here is the Casper-related hack in zproject/test_settings.py,
which was in place before my change and remains
after it:
if CASPER_TESTS:
WEBPACK_FILE = 'webpack-stats-production.json'
else:
WEBPACK_FILE = os.path.join('var', 'webpack-stats-test.json')
I added similar logic in tools/webpack:
if "CASPER_TESTS" in os.environ:
build_for_prod_or_casper(args.quiet)
I also made the helper functions in `./tools/webpack` have
nicer names.
So, now tools should all be able to run standalone and not
rely on previous tools creating webpack stats files for
them and leaving them in the file system. That's good.
Things are still a bit janky, though. It's not completely
clear to me why `test-js-with-casper` should work off of
a different webpack configuration than the other tests.
For now most of the jankiness is around Casper, and we have
hacks in two different places, `zproject/test_settings.py` and
`tools/webpack` to force it to use the production stats
file instead of the "test" one, even though Casper uses
test-like settings for other things like which database
you're using.
We don't use input.create_non_editable_pill() in our
code yet. If we add this back, we'll want to have node
tests on it.
Removing this unused code brings us to 100% line
coverage for input_pill.js.
This directly reverts 5c11ab85 with the small addition
of adding input_pill to our list of fully covered
modules.
These repositories (`zulip-ios-legacy` and `zulip-android`) are
deprecated, and as such should not have their own tabs, but still
should be included in the total contributions count.
Apparently, `puppet-lint` on Ubuntu trusty throws warnings for certain
quoting patterns that are OK in modern `puppet-lint`. I believe the
old Zulip code was actually correct (i.e. the old `puppet-lint`
implementation was the problem), but it seems worth changing anyway to
suppress the warnings.
We also exclude more of puppet-apt from linting, since it's
third-party code.
Instead of using a hardcoded value for spritesheet dimensions,
automatically calculate it using `emoji_data`. This will free
us from updating it only emoji datasource update as well as
allow us to add google blob emojiset.
Since this class was built, folks have always chosen
to subclass JsonableError for situations where
the default of ErrorCode.BAD_REQUEST is insufficient.
So now we simplify the use cases, which also gets
us 100% coverage on this core module.
For building Zulip in an environment where a custom CA certificate is
required to access the public Internet, one needs to be able to
specify that CA certificate for all network access done by the Zulip
installer/build process. This change allows configuring that via the
environment.
We start to use puppet-lint to lint puppet modules by default by
adding it to tools/lint (which controls our linter tool chain).
We also define a few puppet-lint rules to exclude.
Fixes: #9185.
This will help us in reducing the size of the release tarball
significantly. I have refrained from changing the `EMOJISETS`
constant in the `emoji_setup_utils.py` as that controls the
emojisets that we want to support. Since we want to re-enable
the feature of changing emojisets sometime again in the future
that variable should be kept as it is as it controls several
other things like emoji scripts that we use to generate emoji
names. Changing it might cause hard to catch bugs.
`emoji-datasource` package v4.0.4 introduced the concept of qualified
and non-qualified emoji codes. As chat programs don't need to use
emoji representation selector, so we used migrated our infrastructure
to use non-qualified emoji codes. But we missed the fact that the
emoji file names in emoji farm are based on emoji data's 'unified'
field and the value of this field has changed. Consequently the image
file names must also have been changed. We used `emoji_code` while
converting the span tags to img tags while processing notifications.
But since now `emoji_code` refers to non-qualified code while image
file names are based on qualified code, we need to rename images
to correctly do the conversion. This commit just fixes this.
Right now it only has one function, but the function
we removed never really belonged in actions.py, and
now we have better test coverage on actions.py, which
is an important module to get to 100%.
This ancient tool predates our practice of collecting test fixtures
for third-party integrations, which is a better general system for the
problem this solved.
After the messages have been imported, set the rendered_content of the
messages instead of leaving its value to be 'None'.
This is important to ensure that:
(1) Performance for users is good after completing the import.
(2) The database's full-text indexes have all of the imported messages
(which only happens properly when Message rows have their
rendered_content field edited).
Fixes#9168.
Now reading API keys from a user is done with the get_api_key wrapper
method, rather than directly fetching it from the user object.
Also, every place where an action should be done for each API key is now
using get_all_api_keys. This method returns for the moment a single-item
list, containing the specified user's API key.
This commit is the first step towards allowing users have multiple API
keys.
In tools/update-locked-requirements line 66:
compile_requirements requirements/prod.in $OUTPUT_BASE_DIR/prod.txt
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/update-locked-requirements line 67:
compile_requirements requirements/dev.in $OUTPUT_BASE_DIR/dev.txt
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/update-locked-requirements line 68:
compile_requirements requirements/mypy.in $OUTPUT_BASE_DIR/mypy.txt
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/update-locked-requirements line 69:
compile_requirements requirements/docs.in $OUTPUT_BASE_DIR/docs.txt
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/update-locked-requirements line 70:
compile_requirements requirements/thumbor.in $OUTPUT_BASE_DIR/thumbor.txt py2
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/travis/production-helper line 24:
if ! apt-get dist-upgrade -y $APT_OPTIONS; then
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/travis/production-helper line 26:
apt-get dist-upgrade -y $APT_OPTIONS
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/test-migrations line 18:
echo "$new_auto_named_migrations" | sed 's/\[[x ]\] / /'
^-- SC2001: See if you can use ${variable//search/replace} instead.
In tools/test-migrations line 27:
echo 'ERROR: Migrations are not consistent with models! Fix with `./tools/renumber-migrations`.'
^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/test-install/destroy-all line 31:
| while read c
^-- SC2162: read without -r will mangle backslashes.
In tools/test-install/install line 57:
installer_dir="$(readlink -f $INSTALLER)"
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/test-install/lxc-wait line 30:
for i in {1..60}; do
^-- SC2034: i appears unused. Verify use (or export if used externally).
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/test-documentation line 6:
echo -e "\e[${color_code}m${message}\e[0m" >&2
^-- SC1117: Backslash is literal in "\e". Prefer explicit escaping: "\\e".
^-- SC1117: Backslash is literal in "\e". Prefer explicit escaping: "\\e".
In tools/test-documentation line 41:
scrapy crawl_with_status documentation_crawler $loglevel
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/test-all-docker line 7:
source /home/zulip/.bash_profile
^-- SC1091: Not following: /home/zulip/.bash_profile: openBinaryFile: does not exist (No such file or directory)
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/test-all line 7:
TEMP=`getopt -o f --long force -- "$@"`
^-- SC2006: Use $(..) instead of legacy `..`.
In tools/test-all line 24:
echo "Running $@"
^-- SC2145: Argument mixes string and array. Use * or separate argument.
In tools/test-all line 26:
printf "\n\e[31;1mFAILED\e[0m $@\n"
^-- SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
^-- SC1117: Backslash is literal in "\e". Prefer explicit escaping: "\\e".
^-- SC1117: Backslash is literal in "\e". Prefer explicit escaping: "\\e".
^-- SC2145: Argument mixes string and array. Use * or separate argument.
^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/start-dockers line 7:
source /home/zulip/.bash_profile
^-- SC1091: Not following: /home/zulip/.bash_profile: openBinaryFile: does not exist (No such file or directory)
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/reset-to-pull-request line 25:
git fetch "$remote" +"pull/$request_id/head":"$target_ref"
^-- SC2140: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A\"B\"C"?
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/provision line 13:
FAIL="\033[91m"
^-- SC1117: Backslash is literal in "\0". Prefer explicit escaping: "\\0".
In tools/provision line 14:
WARNING="\033[93m"
^-- SC1117: Backslash is literal in "\0". Prefer explicit escaping: "\\0".
In tools/provision line 15:
ENDC="\033[0m"
^-- SC1117: Backslash is literal in "\0". Prefer explicit escaping: "\\0".
In tools/provision line 19:
PARENT_PATH=$( cd "$(dirname "${BASH_SOURCE}")" ; pwd -P )
^-- SC2128: Expanding an array without an index only gives the first element.
In tools/provision line 32:
if [ $failed = 1 ]; then
^-- SC2086: Double quote to prevent globbing and word splitting.
In tools/provision line 49:
echo 'or just close this shell and start a new one (with Vagrant, `vagrant ssh`).'
^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/optimize-svg line 3:
if [ `node_modules/.bin/svgo -f static/images/integrations/logos | grep -o '\.[0-9]% = ' | wc -l` -ge 1 ]
^-- SC2046: Quote this to prevent word splitting.
^-- SC2006: Use $(..) instead of legacy `..`.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/find-unused-css line 5:
if [ $(git grep "$n" | grep -v '^static/styles/zulip.css' | wc -l) -eq 0 ]; then
^-- SC2046: Quote this to prevent word splitting.
^-- SC2126: Consider using grep -c instead of grep|wc -l.
In tools/find-unused-css line 6:
echo $n
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/fetch-rebase-pull-request line 15:
git checkout -B "review-${request_id}" $remote/master
^-- SC2086: Double quote to prevent globbing and word splitting.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
In tools/do-destroy-rebuild-test-database line 6:
"`dirname "$0"`/../tools/setup/generate-fixtures" --force
^-- SC2006: Use $(..) instead of legacy `..`.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
We probably should have done this a while ago, even
though these functions are pretty tiny. The goal here
is to make it easier to have more consistent search
semantics.
Our first use case is subs.js. In this case we
are able to decouple a bit of generic string
matching from the subs-specific code.
I often find myself looking manually through the reflog of `master` to
find a commit I previously reset to with tools/reset-to-pull-request .
Sometimes I want to see a previous version of a PR I'm reviewing a
revised version of; sometimes to look at two related PRs together.
So, here's a feature to automate that by saving each PR branch in its
own ref, with a name like `refs/remotes/pr/1234` -- or `pr/1234`, as
you'd normally refer to it.
To enable this, set the new config option:
$ git config zulip.prPseudoRemote pr
(Or you can pick another name.)
The reason I hesitate to just make this the behavior for everyone
immediately is that the resulting `pr/1234` refs will naturally
accumulate and may clutter up the view -- and because with the
`refs/remotes/` style of name I've chosen, it requires a bit of
Git plumbing to clean them up. (Use `git update-ref -d`.)
I'll play with it and iterate; comments welcome from other willing
early adopters.
This commit closes a long pending issue which involved moving the
`EMOTICON_CONVERSION` mapping to build_emoji infrastructure so
that there is only one source of truth. This was pending from the
time when this feature was implemented.
This commit updates the `emoji-datasource` packages to version 4.0.4.
This update brings following changes to emoji infra:
1: Fix for the bleeding sprite sheets.
2: The category of some emojis has been changed. Categorywise breakup of
net gain or loss is as follows:
Travel & Places: 58 (gain)
Symbols: 47 (loss)
Smileys & People: 52 (gain)
Objects: 11 (loss)
Food & Drink: 3 (gain)
Animals and Nature: 46 (gain)
Activities: 9 (loss)
3: There were some changes in the image farm of the package which were
breaking our old emoji farm. I fixed them by modifying the remapped
emoji map.
Fixes: #8235.
Google emojiset's octopus is really cute and whole Zulip community
loves it. So using a CSS hack, we hardcode octopus emoji to use image
from Google's emojiset only irrespective of the choosen emojiset.
Our CSS checker globs for .css files. Since the
SCSS cutover, it has been a no-op, so there's no
sense launching it. See #8894 for details on
future plans.
This migrates Zulip to use a dramatically better set of names and
aliases for our emoji set, defined in emoji_names.py (which is in turn
manually generated from our hand-curated CSV file).
This should significantly improve the experience of using Zulip's
emoji picker and emoji typeahead for finding what one is looking for.
Credits to @rishig, Alice Lai, and @rntharu for naming all the emoji.
Names are inspired by iamcal, gemoji, and unicode names, sources like
emojipedia and iemoji, google search results for articles about emoji,
and emoji usage on twitter.
We were already correctly including libssl-dev in Zulip's dependencies
in development environment provisioning, but (at least now) it's
needed to build certain Python packages like pycurl when building a
Zulip virtualenv in production. I haven't investigated why we didn't
need this on Ubuntu, but one possible reason would be that some other
library in our dependencies list happens to depend on it on Ubuntu.
We fix this by moving the dependency over to the shared
VENV_DEPENDENCIES list.
Fixes part of #9946.
This will allow us to begin to add our own stubs for external
libraries. Writing stubs can be surprisingly little work to do, and
can have high leverage in keeping our type annotations high-quality.
This implements right-to-left message automatic detection support in
the compose box as well as the message feed. Full unit tests and
support in the message-editing UI are for future work (as are
potentially more fancy things like supporting things like
right-to-left multi-word names for users/streams/etc.).
Fixes#3123.
This will be helpful in the upcoming changes which will make use
of this extracted function to re-create zulip_test_template after
migrating zulip_test db so that we have latest schema in tests.
This commit fixes some modules that were erroneously left out while
transitioning app.js to webpack. This commit exposes them using
expose-loader or setting them directly to window.
This commit moves all files previously under the 'app' bundle in
the Django pipeline to being compiled by webpack under the 'app'
entry point. In the process, it moves assets under the app entry
to a file called app.js that consumes all relevant css and js files.
This commit also edits the webpack config to be able to expose certain
variables for third party libraries that are currently required by
some modules. This is bad coding form and should be refactored to
requiring whatever dependencies a module may have; we're just
deferring that to the future to simplify the series of transitions we
need to do here. The variable exposure is done using expose-loader in
webpack.
The app/index.html template is edited to override the newly introduced
'commonjs' block in the base template. This is done as a temporary
measure so as not to disrupt other pages on the app during the transition.
It also fixes the value of the 'this' context that was being inferred
as window by third party libraries. This is done using imports-loader
in the webpack config. This is also messy and probably isn't how we
want things to work long term.
The only changes visible at the AST level, checked using
https://github.com/asottile/astpretty, are
zerver/lib/test_fixtures.py:
'\x1b\\[(1|0)m' ↦ '\\x1b\\[(1|0)m'
'\\[[X| ]\\] (\\d+_.+)\n' ↦ '\\[[X| ]\\] (\\d+_.+)\\n'
which is fine because re treats '\\x1b' and '\\n' the same way as
'\x1b' and '\n'.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Adds search_pill.js to the static asset pipeline. The items
for search pill contain 2 keys, display_value and search_string.
Adding all the operator information i.e the operator, operand and
negated fields along with the search_string and description was tried out.
It was dropped because it didn't provide any advantage as one had to
always calculate the search_string and the description from the operator.
The appropriate name for the remote pointing at the repo we maintain
may be `upstream` for most of our repos... but not when we're
downstream of someone else, e.g. for react-native. So, make it easy
to configure per-repo.
This results in a significant optimization in the performance of
re-provisioning Zulip if all that you're doing is rebasing onto a
newer version of master (which just adds new migrations).
The change carries some risk of generating unpleasant-to-debug
situations, because if we merge a buggy migration and then later fix
it, some clients may not have a properly migrated database (and also,
this changes how populate_db commutes with migrations). But it seems
worth it, given how much time is currently wasted by not having this.
Fixes: #9512.
In this commit we are adding run_generate_fixtures_if_required,
a new function which is meant to de-duplicate a bit of code
between test-server and test-backend which is essentially
responsible for rebuilding the test database if that was required.
In this commit we are essentially just refactoring the function
is_template_database_current to be called template_database_status
and adjusting the return values accordingly.
This is essentially a preparatory commit for the upcoming commits
which will essentially enable us to not throw away entire DB and
rebuild from scratch if only running migrations could do the job.
This should avoid us creating duplicate webpack bundles every time we
do a deployment, even if none of the files in the bundles themselves
have changed at all.
This option (aka `--raw-output`) prints a string as itself, rather
than JSON-encoded; which makes it fit a bit better in a shell script,
saving us a layer of quoting.
This replaces ad4617c95 with a different fix for the same issue:
instead of stripping the `.git` off separately, we can just correct
the regex, using `+?` to fix our stepping in a classic regex pitfall.
This is a performance optimization: Rather than copying these files
into the `prod-static` directory and then deleting them, we just don't
copy them over in the first place.
For styles, it might have once been the case that this did something,
but we've moved them all to being managed by webpack some time ago.
For the js directory, I think it was never useful to copy and then
delete them; these files were always compiled via tools/minify-js,
and the raw JS files weren't needed, anyway.
This changes run-dev.py to ensure that we have in fact compiled
handlebars templates before running webpack, which is the right model.
Future work will likely include running the handlebars compiler from
webpack, and thus eliminating this extra process.
This improves the performance of these operations, by saving a ~50ms
Python process startup. While not a major performance improvement, it
seems worth it, given how often these commands get run.
Fixes#9571.
First, it's silly that these weren't in common.css in the first place,
since that meant these were a bunch of duplicated code, but
additionally, that meant that these weren't available on the
`/activity` page (or other pages that don't include the portico styles).
Fixes#9561.
Makes the i18n strings in this file much easier to translate by splitting
them into smaller chunks (which avoids having a lot of code in the tagged
strings), and adds a string that was missing as well.
We fix the issue of check-templates spitting out diff between
expected and found indentation of a file before mentioning the
error message and the file name. Basically stuff was being in the
wrong order despite the fact that in code stuff was happening in the
correct order ie, first print the error message along with the filename
and then the actual diff between expected and found file indentation.
Fixes: #9533.
A "zform" knows how to render data that follows our
schema for widget messages with form elements like
buttons and choices.
This code won't be triggered until a subsequent
server-side commit takes widget_content from
API callers such as the trivial chat bot and
creates submessages for us.
This starts the concept of a schema checker, similar to
zerver/lib/validator.py on the server. We can use this
to validate incoming data. Our server should filter most
of our incoming data, but it's useful to have client-side
checking to defend against things like upgrade
regressions (i.e. what if we change the name of the field
on the server side without updating all client uses).
I mistakenly pushed a PR when my tests failed. I ran with
the coverage option, so I saw this brightly colored summary
report that distracted me from the failure message.
This adds a couple newlines and some all caps.
The timezone environment variable was set to UTC initially. It was
changed to something other than UTC so that any local vs UTC
conversion issues will manifest in the tests.
Fixes: #5105.
We essentially stop running create_realm_internal_bots during
every provisioing and move its operations to run from populate db.
In fact to speed things up a bit we actually make populate db call the
funcs which create_realm_internal_bots calls behind the scenes.
Fixes: #9467.
This is required because the --settings=zproject.test_settings param
doesn't work with migrate or the dumpdata management commands. Thus
untill now if one ran just this tool ended up with test database not
properly setup. We never noticed this because test-backend ran this
tool again (after exporting DJANGO_SETTINGS_MODULE) thus making the
tool work this time.
I've often done this by hand -- basically typed out the last line,
with the variables found from looking at the PR page in a browser.
Seems nicer for both us maintainers and the contributor, in particular
because the PR gets marked as merged, instead of closed. But it's a
bit of a pain, and I do it maybe half the time or less; plus it's kind
of a subtle GitHub feature, and as a result I think other maintainers
of Zulip repos do this approximately never.
I've always figured this couldn't be hard to automate; today I decided
to take the 45 minutes to look up how, write out the script, QA it,
write up a nice usage message and some comments, and commit it. :)
This commit improves the output that blueslip produces while
showing error stack traces on the front-end. This is done by
using a library called error-stack-parser to format the stack
traces.
This commit also edits the webpack config to use a different
devtool setting since the previous one did not support sourcemaps
within stack traces. It also removes a plugin that was obviated
by this change.
This improves test coverage for a lot of our webhooks that relied
on ad-hoc methods to handle unexpected event types.
Note that I have deliberately skipped github_legacy, it isn't
advertised and is officially deprecated.
Also, I have refrained from making further changes to Trello, I
believe further improvements to test coverage should be covered
in separate per-webhook commits/PRs.
UnexpectedWebhookEventType is a generic exception that we may
now raise when we encounter a webhook event that is new or one
that we simply aren't aware of.
Now that we have tsearch_extras packages uploaded, this mostly works.
There's a few issues being debugged in #9460; they should be fixed
soon, and regardless, merging this will make it easier to develop.
This makes it possible to again use the *.zulipdev.com domains in the
development environment.
Ideally, we'd also read REALM_HOSTS to make this more flexible.
This adds a tour of Zulip to the bottom of the homepage.
In order to get the carousel nave, we use Bootstrap 2 from a CDN on
this page; this isn't ideal in the medium term, but upgrading
Bootstrap across the project is too much work for now.
Apparently, we were incorrectly appending each new hash onto the end
of the file, basically resulting in every run of provision being
treated as a miss for this cache.
Fixing this saves about 4s (over 1/3) of the no-op provision time.
Fixes#9233.
Uses nargs='*' instead of nargs='argparse.REMAINDER'.
nargs='argparse.REMAINDER' gathers remaining terms as arguments
even if it is an option e.g --coverage, while '*' gathers all the
command-line arguments until the next option is encountered.
The only slash command implemented in this initial
version is an extremely crippled version of a
"/stats" slash command that reports that you are
running 1 server.
This has a cool structure, but it's written against the long-dead
South API, and we can always pull it out of the Git history if we want
to use this approach in the future.
This leaves the wrapper script with very little left to do!
The main thing left is finding scripts by searching for shebang lines;
mypy itself would happily do the search for importable Python files.
Cleaned up add_user_list_args(). The "help" and
"all_users_help" have all default values. As noted in
an earlier commit, "all_users_help" is always passed in,
so we can get rid of "all_users_arg". We keep the default
for "all_users_help" so we don't have to change variable order
in function definition.
Since the region of base.zulipdev.org is
NYC3 we had to add SFO1 as an additional
region each time a snapshot of base droplet
is created. This is required as droplets
can be created in SFO1 only if there is
an image present in that region. Adding
of droplet image to SFO1 takes a lot of
time as well as cost 2X as we are storing
2 images. It's better to just create new
droplets in NYC3 instead. Alternatively we
can create a new base droplet in SFO1 if
we want all the droplets to be created in
SFO1.
This puts all of this config in one place, and also needs a lot fewer
lines to describe it; which, combined, makes it a lot clearer what our
normal config actually is. (I'd been looking at this script for a few
minutes without realizing that we have `--disallow-untyped-defs` *on*
by default, not off.)
Experimenting with different values is still easy; just comment the
line in the config.
This commit moves the stylesheets under the archive bundle in
the Django pipeline to being compiled by webpack instead. It
also removes a remaining call to a portico stylesheet that no
longer exists.
This commit transitions landing-page.css from the Django pipeline
to being compiled by webpack as landing-page.scss under the
'landing-page' and 'integration' bundles.
This commit transitions all styles in app.css in the Django pipeline
to being compiled by webpack in an app-styles bundle, and renames the
various files to now be processed as SCSS.
To implement this transition, we move the old CSS file refernces in
settings.py and replace them with a bundle declared in
`webpack.assets.json` and includedn in the index.html template
Tweaked by tabbott to keep the list of files in `app.css` in
`webpack.assets.json`, and to preserve the ordering from the old
`settings.py`.
This commit removes the flash on unstyled content while in dev
mode that was caused by the use of style-loader. Instead it
enables mini-css-extract-plugin in dev in combination with
css-hot-loader which enables HMR for development.
This is because mini-css-extract-plugin does not currently support
HMR out of the box. It also adds a SourceMapDevtoolPlugin to enable
sourcemaps with css since mini-css breaks sourcemaps when used in
combination with the cheap-module-evel-source-map setting.
Related issues:
https://github.com/webpack-contrib/mini-css-extract-plugin/issues/34https://github.com/webpack-contrib/mini-css-extract-plugin/issues/29
We modify check-templates to check for duplicate id's in archive
templates and app templates separately. This means we are allowing
app and archive templates to potentially use same id's. This is
needed because we intend to re use some js from the main app and
having same id's help achieve that.
Note: We haven't up until this point actually added archive
templates. This commit is more of a preparatory commit for merging
the basic archive infra.
Adds support for importing png files using file-loader in webpack.
Changes the name of the output directory to be files instead of
fonts for better readability.
This commit removes the need for portico.css to be generated
by the Django pipeline and makes the error page use the css
file compiled by webpack instead.
Combines, both portico js and css into one bundle. This for now solve
the issue of an empty js bundle being generated by webpack for the
portico-styles stylesheet.
This was a bit more than moving code. I extracted the
following things:
$widget (and three helper methods)
$input
text()
empty()
expand_column
close_widget
activity.clear_highlight
There was a minor bug before this commit, where we were inconsistent
about trimming spaces. The introduction of text() and empty() should
prevent bugs where users type the space bar into search.
This commit adds a --quiet argument to tools/webpack which removes
the verbose output from webpack and replaces it with showing only
errors. It also makes tools/run-dev --tests use this argument while
running webpack for testing.
Tweaked by tabbott to clean up the code a bit.
static/styles/scss/portico.scss is now compiled by webpack
and supports SCSS syntax.
Changed the server-side templates to render the portico-styles
bundle instead of directly requiring the portico stylesheet. This
allows webpack to handle stylesheet compilation and minification.
We use the mini-css-extract-plugin to extract out css from the
includes in webpack and let webpacks production mode handle
minification. Currently we're not able to use it for dev mode
because it does not support HMR so we use style-loader instead.
Once the plugin supports HMR we can go on to use it for both
dev and prod.
The downside of this is that when reloading pages in the development
environment, there's an annoying flash of unstyled content :(.
It is now possible to make a change in any of the styles included
by static/styles/scss/portico.scss and see the code reload live
in the browser. This is because style-loader which we currently
use has the module.accept code built-in.
This commit fixes hot module replacement in webpack. To do this
we open port 9994 used by webpack to communicate between browser
and devserver. The attempts to forward the proxy from 9991 failed
so the last resort was to open up the webpack port.
It also removes an uncessary plugin in the webpack config and moves
the --hot flag to tools/webpack.
We've already got a bunch of other comments on work we need to do for
this decorator and an open issue that will ensure we at some point
rework this and add tests for it. In the meantime, I'd like to lock
down the rest of decorator.py at 100% coverage.
Fixes#1000.
Webpack dev server by default does host checking for requests. so
in dev enviorment if the the request came for zulipdev.com it would not
send js files which caused dev envoirment to not work.
This should make it easier to find the templates that are actually
part of the core webapp, instead of having them all mixed together
with the portico pages.
Update the config file to show slightly more information while
compiling webpack. Also decreased the time webpack waits before
recompiling in order to speed up HMR.
Changed the devtoool setting for development from 'eval' to
'cheap-module-eval-source-map' as it has better support for
breakpoints in Google Chrome and the difference is time is
negligible at the number and size of files currently being
consumed by webpack. This stragtegy can be reviewed in the
future as the size of files grows or Chrome adds better
support.
Upgrade webpack to latest version at the time of authoring. This
involves upgrading webpack version and its loaders to compatible
versions. It also involved editing tools/webpack to use the
executable for webpack-cli instead because of a change in how the
webpack package wants you to handle shell execution.
It also fixes the confugration for TypeScript in the webpack config
as that was previously broken. Including TypeScript files in JS
files compiled by webpack now works.
This prevents us from using const in our JS code, with exceptions
for test code and the portico. Hopefully this is just a temporary
rule until we make our pipelines with work with ES6.
I tried to prevent "let", but that was too noisy.
This adjusts the one false-negative case of using const in a comment.
We now allow you to run --coverage on individual files. This helps
when you want to make sure a file is being covered directly and not
just getting incidental coverage from higher level tests.
Before this commit, we were conflating wanting coverage reports with
wanting coverage checks. For individual files, we now solve that by
simply eliminating the coverage checks. This required some minor
refactoring to extract some functions.
This fixes a bug where provision was failing since our most recent
upgrade to yarn/nvm/node.
It turns out my original fix was the correct fix, but to the wrong
third-party tool: nvm, not yarn, was the offender.
String.prototype.endsWith is not supported in ie11.
Adds string.prototype.endswith package to dependencies and places
it at `common` entry point in webpack.assets.json.
Fixes#8944.
Adds string.prototype.startswith package to dependencies and places
it at `common` entry point in webpack.assets.json. As common.js is
loaded on all code paths first, there is no need to place this package
into other entry points.
We make some specific cases of tags use 2 space indents.
The case description:
* A tag with opening tag spread over multiple lines and closing tag
on the same line as of the closing angle bracket of the opening tag.
* A tag with opening tag spread over multiple lines and closing tag
not on the same line as of the closing angle bracket of the opening
tag.
Example:
Case 1:
Not linted:
<button type="button"
class="btn btn-primary btn-small">{{t "Yes" }}</button>
After linting:
<button type="button"
class="btn btn-primary btn-small">{{t "Yes" }}</button>
Case 2:
Before linting:
<div class = "foo"
id = "bar"
role = "whatever">
{{ bla }}
</div>
After linting:
<div class = "foo"
id = "bar"
role = "whatever">
{{ bla }}
</div>
After some thinking, I don't think there's any actual value to doing
the ../ style relative links here, whereas there is actual harm from
the links being slightly broken in the current model. We fix this by
just using /#settings as the URL.
Fixes#8978.
Added support for passing a filename without `.js` suffix.
This then fixed the issue of no complaints for invalid test
files. Now, throws an error for invalid test files.
Fixes#8579.
Update perfect-scrollbar to fix stutter space-scrolling in #8544. Also
reworked deprecated `element.perfectScrollbar` to `new
PerfectScrollbar(element)`. Lastly, updated provision version and
changed node module path to new path.
This also refactors perfect-scrollbar in help.js to work with updated
version of perfect-scrollbar. Because the update also changed
perfect-scrollbar's css selectors for all scrollbars in zulip, we
update those too.
Fixes#8544.
Namely, annotate as best as possible, and add notes to indicate preference,
if QuerySet develops generic typing.
Note that the return values of functions with annotations changed in this
commit are used elsewhere as QuerySets, so the Sequence[T] approach used
for some functions in models.py is not applicable.
Other functions took the form of returning Sequence[T] when the QuerySet
functionality is unused beyond the function, with T being the objects
filtered for in the function body; this commit follows that practice for the
one remaining python2 comment-annotated function, completing the transition
of models.py to py3.5 function annotations.
A note is also added to another function regarding a need to return a
QuerySet, and ideally a QuerySet[T] in line with the other functions, as and
when QuerySet becomes annotated as a generic.
This commit switches our emoji infrastructure to use 256 color indexed
64px spritesheets. Earlier we were using non-indexed 32px spritesheets
which were blurry on high dpi displays. These indexed spritesheets not
only provide a crispier display but are also smaller in size.
This commit also removes the `emoji-datasource` package as a dependency
as all the data is now sourced from individual datasource packages.
Fixes: #7862.
Main exceptions are scripts/tools/puppet & tests.
Other current exclusions:
- api_test_helpers.py (avoid changing test code);
- cache.py, due to comments in file;
- models.py, due to failure on QuerySet[Message];
- stream_subscription.py, due to failure on QuerySet[Subscription];
- tornado/descriptors.py;
- views/streams.py, due to remaining FuncKwargPair issue;
- zthumbor/, since thumbor is in python2.
Tweaked by tabbott to partially document the stauts in comments.
This flips the experimental `--express` option to be the default.
We retain the old behavior, where the script exits before
`initialize-database`, as an option `--no-init-db`; it might be useful
in e.g. a migration scenario (from a Zulip install elsewhere, or
another chat system) where the admin wants to set up the database
separately.
The install instructions are adjusted to match, getting shorter by two
steps and a bunch of words. I think this opens up opportunities to
refactor the text to simplify things further, too, but leaving that
for another commit.
Also tweak the "production" test suite to match.
This replaces the cumbersome system we had for giving users feedback
on settings state changes in the display settings UI.
We expect this new system to be what we will attempt to migrate other
settings widgets to match over the coming weeks and months. It also
provides the opportunity to significant refactor away a lot of the
code duplication in settings_display.js.
Thanks to Brock Whittaker for redoing the styling and improving the
code simplicity.
Fixes#7622.
These two classes are tricky to test, and nocoverage-ing them
allows us to mark queue_processors.py as fully covered. We
still want to cover these two workers at some point, but for
now, it's nice to enforce full coverage for any future changes
to queue_processors.py.
Fixes (sort of) #6542.
This should help prevent against bugs where we accidentally introduce
use of sudo somewhere in the production installer or upgrade code path
(these used to happen all the time), which doesn't work on production
systems that don't have sudo setup.
Revert c8f034e9a "queue: Remove missedmessage_email_senders code."
As the comment in the code says, it ensures a smooth upgrade path
from 1.7.x; we can delete it in master after 1.8.0 is released.
The removal commit was merged early due to a communication failure.
This commit adds tests for the fixture for when a user is not
authorized (perhaps because the query requires the use of admin
privileges) for a particular query.
In templates/zerver/api/update-message.md, we have a sample fixture
for when a zulip.Client does not have the permission to update/edit
a particular message. This commit adds a test for that fixture.
Also, tools/test-api now also uses a non-admin client for this test,
which might come in handy in the future.
We now isolate the code to transmit messages into transmit.js.
It is stable code that most folks doing UI work in compose.js don't
care about the details of, so it's just clutter there. Also, we may
soon have other widgets than the compose box that send messages.
This change mostly preserves test coverage, although in some cases
we stub at a higher level for the compose path (this is a good thing).
Extracting out transmit.js allows us to lock down 100% coverage on that
file.
In this commit we add support for some tags which are also called
void-elements according to
http://w3c.github.io/html/syntax.html#void-elements to be parsed by
our template parser and get tagged as singleton_html_tags.
Fixes: #8387.
Now executable! Just run `tools/tagmessages`.
Also, get the username and password from a `.transifexrc` file.
And hardcode the project slug to `zulip-test` rather than to `zulip`;
the Transifex API is bad at namespacing, so this makes it possible to
run this script on a test project (the only way we're currently using
it) even for people like me who can also upload to the real Zulip
project on Transifex.
We now have a separate page for common error payloads, for example,
the payload for when the client's API key is invalid. All error
payloads that are presented on this page will be tested similarly
to our other non-error sample fixtures.
Otherwise prepare-base is likely to fail when first run (but then
succeed when rerun, because the container is left running), because
the container isn't up yet when we try to operate in it.
Also clean up the placement of `set -e` vs `set -x`.
Apparently, we've now had the first time one of our contributors had
their account deleted (at least, the author page for the contributor
who has 21 commits in python-zulip-api now 404s).
See 625939 for more information. In short, the purpose of this delay is
to give autoreload code enough time to touch every watched file at least
once before the change is made.
This script and our CI scripts tools/travis/{backend,frontend} have
stayed pretty well in sync in the 6 months since 360c27ded made that
relationship explicit and easy to check!
Just one small exception; so fix that.
This may or may not be temporary, but either way, the other code is
there in source control, and the "why" of disabling gitlint is the
helpful bit for a comment.
Injecting the generated-file warning into the settings dict felt a
little unnecessarily magical. A warning like this is always going
to be at the top; the way it might differ between files is mainly
if the syntax for a comment varies, and in that case a simple
substitution like we're doing in this template wouldn't be enough
to express the difference anyway. So, embrace the hardcoding.
Now, the template and the images.yml entry have a very simple
relationship: the keys in one are exactly the keys in the other.
That's good for people quickly and confidently understanding it.
Now that the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples depended on code in the
tools/lib/* directory, it caused the production tests to fail since
the tools/ directory wouldn't exist in a production environment.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_example to generate the example
fixture and code example, so that both are tested in
tools/lib/api_tests.
We don't disable code by commenting it out -- that leaves a mess.
We delete it. Remembering what the code was is what source control
is for.
This fixes cd849bc3f "test-run-dev: Disable Nagios check."
from a few weeks ago.
For now, this does nothing in a production environment, but it should
simplify the process of doing testing on the Thumbor implementation,
by integrating a lot of dependency management logic.
This commit also adds a tool to push translation sources to Transifex.
This tool makes sure that we don't push mobile source file. Mobile
source file is supposed to be handled from Zulip-Mobile repo.
(This is a small fixup to the main change, which was accidentally
included in a previous commit:
08bbd7e61 "settings: Slightly simplify EMAIL_BACKEND logic."
Oops. See there for most of the changes described here.)
The installer works out of a release-tarball tree. We typically want
to share this tree between successive test-install runs (with an rsync
or similar command to update source files of interest) because
rebuilding a release tree from scratch is slow. But the installer
will munge the tree; so instead of directly bind-mounting the tree
into the container, we need to give it an overlay over the tree, as a
sandbox to play in.
Previously we used lxc-copy's `-m overlay=...` feature to do this,
mounting an overlay in the container. But then sometimes in
development we want to reach in and edit some code in the tree,
e.g. before rerunning the installer after something failed. Reaching
inside the container for this is a pain (`ssh` would add latency, and
I haven't installed sshd in the containers; and getting rsync to work
with `lxc-attach` was beyond what I could figure out in a few minutes
of fiddling); and editing the base tree often doesn't work.
So, create the overlay with our own `mount -t overlay`, and have
`lxc-copy` just bind-mount that in. Now the host has direct access to
the same overlay which the guest is working from.
Also this makes it past time to help the user out in finding the fresh
names we've created: first the container, now this shared tree. Print
those at the end, rather than make the user scroll to the top and find
the right `set -x` line to copy-paste from.
This is the last commit in the series of commits for completing the
project of cleaning up our html templates to have 4 space and
valid indentation.
Fixes: #1236.
In this commit we also fix a test which would fail as a result of
doing this cleanup since the test wasn't designed to take into
account the space chars which might occur in the beginning of a
html line.
This test randomly fails far too often in Travis -- I think more than
all our other tests combined. It needs to be fixed before we can ask
everyone to look at build failures it causes.
Now that we have `eval_rst` and can explicitly exclude pages from the
toctree completely, we no longer need to set `includehidden`, and we
can return to using upstream's template.
(Meanwhile, our feature request upstream was successful! See
rtfd/sphinx_rtd_theme#485, which upstream implemented just a week
after we requested it. So that would have been another option.)
This reverts commit 11b8b8f48 "docs: Add rtd layout template."
It runs in kind of a peculiar environment -- in particular with the
`tags` identifier injected into the namespace -- and it contains
very little code more complex than `foo = "bar"`, so there's not
much to check anyway.
Before this fix, the installer has an extremely annoying bug where
when run inside a container with `lxc-attach`, when the installer
finishes, the `lxc-attach` just hangs and doesn't respond even to
C-c or C-z. The only way to get the terminal back is to root around
from some other terminal to find the PID and kill it; then run
something like `stty sane` to fix the messed-up terminal settings
left behind.
After bisecting pieces of the install script to locate which step
was causing the issue, it comes down to the `service camo restart`.
The comment here indicates that we knew about an annoying bug here
years ago, and just swept it under the rug by skipping this step
when in Travis. >_<
The issue can be reproduced by running simply `service camo restart`
under `lxc-attach` instead of the installer; or `service camo start`,
following a `service camo stop`. If `lxc-attach` is used to get an
interactive shell, these commands appear to work fine; but then when
that shell exits, the same hang appears. So, when we start camo
we're evidently leaving some kind of mess that entangles the daemon
with our shell.
Looking at the camo initscript where it starts the daemon, there's
not much code, and one flag jumps out as suspicious:
start-stop-daemon --start --quiet --pidfile $PIDFILE -bm \
--exec $DAEMON --no-close -c nobody --test > /dev/null 2>&1 \
|| return 1
start-stop-daemon --start --quiet --pidfile $PIDFILE -bm \
--no-close -c nobody --exec $DAEMON -- \
$DAEMON_ARGS >> /var/log/camo/camo.log 2>&1 \
|| return 2
What does `--no-close` do?
-C, --no-close
Do not close any file descriptor when forcing the daemon
into the background (since version 1.16.5). Used for
debugging purposes to see the process output, or to
redirect file descriptors to log the process output.
And in fact, looking in /proc/PID/fd while a hang is happening finds
that fd 0 on the camo daemon process, aka stdin, is connected to our
terminal.
So, stop that by denying the initscript our stdin in the first place.
This fixes the problem.
The Debian maintainer turns out to be "Zulip Debian Packaging Team",
at debian@zulip.com; so this package and its bugs are basically ours.
This is a tool that throws away `fsync` calls and other requests for
the system to sync files to disk. It may make the install faster; for
example, if it has to install a number of system packages, `dpkg` is
known to make a lot of `fsync` calls which slow things down
significantly. Conversely, if there's a power failure in the middle
of running a test install, we really don't mind if the test install's
data becomes corrupt.
When the install script is successful, one of the final things it
wants to do is to move the tree that Zulip was installed from into the
deployments directory. It can't do that, at least not in a naive way
with `mv`, if the tree is actually a mount point. So, stick the tree
inside some other directory that we create just for the purpose of
being the mount point and containing the install tree.
This saves several minutes off the install time. Sadly pip still
clones Git repos for dependencies that point to them, but for many
others (not all? not sure) it just gets a wheel from the cache.
The third-party `install-yarn.sh` script uses `curl`, and we invoke it
in `install-node`. So we need to install it as a dependency.
We've mostly gotten away with this because it's common for `curl` to
already be installed; but it isn't always.
This greatly simplifies iterating on changes to the installer and
associated code: just edit in the shared directory (or edit in your
worktree and rsync to the directory), and rerun.
With this change, the form with a directory is now really the main
way to run the script; the form accepting a tarball is really just
a convenience feature, unpacking the tarball and then proceeding with
that directory.
This will facilitate testing interesting installer features
using its own CLI.
On my laptop, with a recent base image (updated a few days ago with
`prepare-base`), it takes just 7 or 8 seconds to get to the installer
running, as timed by passing `--help` so that the installer promptly
exits.
In order to do development on the installer itself in a sane way,
we need a reasonably fast and automatic way to get a fresh environment
to try to run it in.
This calls for some form of virtualization. Choices include
* A public cloud, like EC2 or Digital Ocean. These could work, if we
wrote some suitable scripts against their APIs, to manage
appropriate base images (as AMIs or snapshots respectively) and to
start fresh instances/droplets from a base image. There'd be some
latency on starting a new VM, and this would also require the user
to have an account on the relevant cloud with API access to create
images and VMs.
* A local whole-machine VM system (hypervisor) like VirtualBox or
VMware, perhaps managing the configuration through Vagrant. These
hypervisors can be unstable and painfully slow. They're often the
only way to get development work done on a Mac or Windows machine,
which is why we use them there for the normal Zulip development
environment; but I don't really want to find out how their
instability scales when constantly spawning fresh VMs from an image.
* Containers. The new hotness, the name on everyone's lips, is Docker.
But Docker is not designed for virtualizing a traditional Unix server,
complete with its own init system and a fleet of processes with a
shared filesystem -- in other words, the platform Zulip's installer
and deployment system are for. Docker brings its own quite
different model of deployment, and someday we may port Zulip from
the traditional Unix server to the Docker-style deployment model,
but for testing our traditional-Unix-server deployment we need a
(virtualized) traditional Unix server.
* Containers, with LXC. LXC provides containers that function as
traditional Unix servers; because of the magic of containers, the
overhead is quite low, and LXC offers handy snapshotting features
so that we can quickly start up a fresh environment from a base
image. Running LXC does require a Linux base system. For
contributors whose local development machine isn't already Linux,
the same solutions are available as for our normal development
environment: the base system for running LXC could be e.g. a
Vagrant-managed VirtualBox VM, or a machine in a public cloud.
This commit adds a first version of such a thing, using LXC to manage
a base image plus a fresh container for each test run. The test
containers function as VMs: once installed, all the Zulip services run
normally in them and can be managed in the normal production ways.
This initial version has a shortage of usage messages or docs, and
likely has some sharp edges. It also requires familiarity with the
basics of LXC commands in order to make good use of the resulting
containers: `lxc-ls -f`, `lxc-attach`, `lxc-stop`, and `lxc-start`,
in particular.
Stripe Checkout means using JS code provided by Stripe to handle
almost all of the UI, which is great for us.
There are more features we should add to this page and changes we
should make, but this gives us an MVP.
[greg: expanded commit message; fixed import ordering and some types.]
This is fairly often -- though not always! -- failing, with a nasty
failure mode where it takes like 6 minutes to time out. See
discussion on #7748 (search for "bad link").
Actually, after seeing it happen just now when running
test-documentation on my laptop, on some other link, it occurs to me
that I've seen this before -- it's fairly common in Travis, too. It's
just that it doesn't actually cause the build to fail :-/, and on
Travis we haven't been paying as close attention to slow builds as we
are on Circle right now.
Generally stderr is the conventional place for this sort of running
commentary, and it's better set up for it: by default stdout may have
a buffer inside the process so that things written to it don't reach
the outside until later, while stderr is always by default unbuffered,
so messages are printed immediately.
Here, until the previous commit, because our color-reset sequence was
being printed without a following newline (with `echo -n`), it was
getting buffered; and then error messages from `scrapy` to stderr were
being erroneously painted with the color intended for the message
"Testing links in documentation...".
The autoreload code of Django works by looping over the files associated
with all the loaded modules. This loop is run after every 1 second. If
the file is found for the first time by the loop, it is assumed that the
file is new and is not modified between the time it is loaded and is
checked by the loop. This assumption is the source of a race condition.
We can either implement a more sensitive version of the loop or we can
just allow enough time to the Django loop to touch every file at least
once.
For the time being, we are going with the second option.
Previously, there were following problems with the implmentation:
* Same file handle was being used to read and write. We used to do
`seek(0)` and then `read()`. This had a chance to overwrite
file data. Now we use different file handles to read and write data.
* We were using text streams. Text streams cannot be used with
`bufferring=0`. Now we use binary streams without buffering so that
data is available for reading without any delay.
This commit also updates the key(s) that we search in the logfile.
Previously, launch of all queues was announced in the log, now we only
anounce the number of threads that were launched.
This commit also makes sure that we always exit after gracefull shutting
down the development server.
Previously, there were following problems with the implmentation:
* Same file handle was being used to read and write. We used to do
`seek(0)` and then `read()`. This had a chance to overwrite file
data. Now we use different file handles to read and write data.
* We were using text streams. Text streams cannot be used with
`bufferring=0`. Now we use binary streams without buffering so that
data is available for reading without any delay.
This commit just copies all the code from MissedMessageSendingWorker
class to a new EmailSendingWorker class. All the logic to send an email
through a queue was already there. This commit only makes the logic
generic. It does so by creating a special purpose queue called
'email_senders' to send any type of email. To make
MissedMessageSendingWorker still work we derive it from
EmailSendingWorker. All the tests that were testing
MissedMessageSendingWorker now run against EmailSendingWorker.
We get the following error (edited slightly):
Dec 19 06:13:27 commit_messages| An error occurred while executing
'/usr/bin/git rev-list --max-count=-1 upstream/master..HEAD':
fatal: ambiguous argument 'upstream/master..HEAD':
unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
We'll need to adjust the remotes to make `upstream` mean what we expect.
This isn't really the right way to do this -- commit
dcd80e665 "travis/setup-backend: Remove the '--travis' flag"
took us in the wrong direction by introducing more magic, deeper in
the stack.
But it's the same way we do it for Travis. For now, just copy that.
[Thanks to hackerkid for cleaning up my original crude hack.]
At this point if we were accidentally using `/srv/zulip-venv` for
anything, we'd have run into it by now. So just drop the bit of
historical logic that we had to ensure that.
This reverts commit 66261f1cc. See parent commit for reason; here,
provision worked but `tools/run-dev.py` would give errors.
We need to figure out a test that reproduces these issues, then make a
version of these changes that keeps that test working, before we
re-merge them.
We should omit these for mypy. For most class definitions,
mypy doesn't need `Any`, and it provides no real useful info.
For clever monkeypatches, you should provide a more specific
type than `Any`.
These commands are super boring standard Docker commands,
so this probably isn't helpful for anyone who is familiar
with building Docker images... but I had to consult docs
to work out the right commands again today, so they'd help me.
Provision was failing at tools/setup/postgres-init-dev-db
with this in the log:
Sep 20 02:27:01 + sudo -i -u postgres psql ''
[sudo] password for circleci:
The issue is that the old version of this line (from Circle upstream)
only lets the `circleci` user sudo to root -- not to other users, or
not directly anyway -- because sudoers syntax is complicated. Fix it,
after studying `man sudoers`.
This is only an optimization -- if this list is missing anything,
we'll get to it in `provision` in the actual build. That's important,
because we want an existing image to work fine for testing new
versions of our codebase, including changes that may install more
packages in `provision`.
What this does accomplish is keeping provision's `apt-get install`
fast, by leaving it very little work to do.
The list comes from looking at the APT output during provision in an
actual run without this step, and leaving out two packages which
aren't available at this stage, because we get them from PPAs:
postgresql-9.3-pgroonga and postgresql-9.3-tsearch-extras.
Install `jq` with APT -- that's a lot simpler to read than this
explicit download.
And coalesce several commands, following Docker upstream's
recommendation and avoiding unnecessary overhead.
This is nearly the same as Circle's version, linked in the comment.
I've
* changed the FROM line to get Ubuntu,
* added a couple of distro packages to compensate, and
* revised the comments.
This seems to have been causing the travis production suite to fail.
It's a direct consequence of removing travis' giant library of apt
sources.list files; now that those are gone, there aren't copies of
all these extra packages available anyway.
This method was new in Tornado 4.0. It saves us from having to get
the time ourselves and do the arithmetic -- which not only makes the
code a bit shorter, but also easier to get right. Tornado docs (see
http://www.tornadoweb.org/en/stable/ioloop.html) say we should have
been getting the time from `ioloop.time()` rather than hardcoding
`time.time()`, because the loop could e.g. be running on the
`time.monotonic()` clock.
tools/setup/generate_zulip_bots_static_files now starts off by
deleting static/generated/bots/ (if it hasn't been removed already)
so that outdated static files from older versions of the zulip_bots
package don't supress errors in the main repo that would otherwise
break.
For more info, see #7542Fixes: #7542.
The `re.match` function in the Python stdlib is a trap for the unwary,
with surprising and asymmetrical semantics; we should probably add a
lint rule to ban it entirely. The docstring says:
> Try to apply the pattern at the start of the string, [...]
In other words, it effectively adds a `^` at the start (or `\A`, where
the distinction matters.) It's bad enough that this differs from what
grep, sed, perl, less, and every other tool I can think of do when
looking for matches to a regex; on top of that, it treats the
beginning of the string differently from the end, for no obvious
reason. The function that does what the rest of the world understands
by "match against this regex" is `re.search`.
In this case, it's unlikely that anyone intended for comments with
URLs, or `api_url` references, to miss out on their respective
exceptions to the long-line rule if they happen to start after the
first column. So fix those rules by just switching to `re.search`
with the same pattern.
I think Markdown URL references may have to start at the beginning of
the line, so I've left a `^` there to preserve -- but now make
explicit -- the `re.match` behavior.
Tweaked by tabbott to move changes from the next commit that are
required for this to pass tests into this commit.
Note that this exports a few items that were not previously exported.
This is part of our efforts to change our integrations/webhooks
docs to follow the same sort of numbered-list format as our /help
docs. In order to indicate that paragraphs separated by newlines
are part of the same numbered-list point, every paragraph must be
indented 4 spaces.
This both improves the comment to be more readable, and also uses the
new and improved exclude feature to limit the exclusion to just the
webhook fixtures (where it's needed).
Also fixes a mypy error.
The previous exclude rules only allowed excluding a directory (and
things in subdirectories would silently still be linted). Anyone
using this would expect it to exclude a directory tree, so we make it
do that.
The readthedocs theme overrides a few settings in their layout template.
We might want to change some settings back to their default values.
This commit copies the original readthedocs layout file from
https://github.com/rtfd/sphinx_rtd_theme/blob/master/sphinx_rtd_theme/layout.html
to _templates/layout.html, and excludes it from lint and template checks.
Addresses #7417.
This fixes a bug where, when a user is unsubscribed from a stream,
they might have unread messages on that stream leak. While it might
seem to be a minor problem, it can cause significant problems for
computing the `unread_msgs` data structures, since it means we need to
add an extra filter for whether the user is still subscribed, either
in the backend or in the UI.
Fixes#7095.
This commit modifies `test-locked-requirements` to use some caching
so that we don't need to use the `update-locked-requirements` tool
everytime for checking the validity of locked requirements as it is
slow.
Fixes: #6969.
This commit renames various source requirements files like `dev.txt`,
`mypy.txt` etc to `dev.in`, `mypy.in` etc and various locked requirements
files like `dev_lock.txt`, `mypy_lock.txt` etc to `dev.txt`, `mypy.txt`
etc. This will help in emphasizing to the user that *.in are actually
input to `update-locked-requirements` tool which should be run after
updating any of these.
This often can cause minor caching problems.
Obviously, it'd be better if we had access to the AST and thus could
do this rule for UserProfile objects in general.
This prevents the caches in /srv from growing to fill up the disk --
e.g., on my laptop after 6 months of regular development the venv cache
was 12G and the NPM cache 5G, making them by far the largest disk hogs
on the machine.
It costs about 0.4s, apart from any time spent actually removing
things. This is a little annoyingly slow to be adding to every
provision, and seems like it could be optimized, but I think already
worth it as is.
We already do this by default in tools/build-docs, but since we
migrated test-documentation to not run that directly (to disable
collapsing), we need to add the recent parallelism fix here too.
It saves about 5-10s when running this test suite for me, which is
good, but definitely leaves me feeling like there could be more
improvement.
In this commit we add new dependencies needed for running thumbor.
Also we add the script for creating the virtual environment ready
for thumbor.
Note: Thumbor will use python2 and thus have different virtualenv
dedicated to it.
Credits to @TigorC and @joshland as well for there work on this.
In this commit we add a new option which could be used to specify
python version. When 'py2' is specified, future/futures are not
removed from the requirements lock file generated.
Even where this is actually used for a temporary checkout, it obscures
the relationship between this and $TMPDIR -- and some of our logic
depends on that. In other places, it isn't actually even a checkout.
In all cases, the expanded version is clearer.
This script, and tools/update-prod-static which it relies on,
have kept getting more complex since this conditional was added
in 2013, and the places we rely on GNU features have probably
multiplied beyond `mktemp -d`. It's unlikely this works on
macOS with BSD tools now, and it'd be hard to maintain that way
if it did; drop the pretense.
There's no need to remove this file here -- the whole tree will be
removed a few commands later, and the `tar` command we do first, to
supplement our tarball with various generated files, is quite
selective and wouldn't look at this file anyway.
With the new portico work we've done, the help documentation does
sorta depend on the database if you're logged in. So it's best to
just require it for these tests.
This commit helps reduce clutter on the navigation sidebar.
Creates new directories and moves relevant files into them.
Modifies index.rst, symlinks, and image paths accordingly.
This commit also enables expandable/collapsible navigation items,
renames files in docs/development and docs/production,
modifies /tools/test-documentation so that it overrides a theme setting,
Also updates links to other docs, file paths in the codebase that point
to developer documents, and files that should be excluded from lint tests.
Note that this commit does not update direct links to
zulip.readthedocs.io in the codebase; those will be resolved in an
upcoming follow-up commit (it'll be easier to verify all the links
once this is merged and ReadTheDocs is updated).
Fixes#5265.
The CSS linter was pretty hard to reason about. It was
pretty flexible about certain things, but then it would
prevent seemingly innocuous code from getting checked in.
This commit overhauls the pretty-printer to be more composable,
where every object in the AST knows how to render itself. It
also cleans up a little bit of the pre_fluff/post_fluff logic
in the parser itself, so comments are more likely to be "attached"
to the AST node that make sense.
The linter is actually a bit more finicky about newlines, but
this is mostly a good thing, as most of the variations before
this commit were pretty arbitrary.
Aman Agrawal
Tim Abbott
Thomas Ip
Vishnu Ks
Wyatt Hoodes
Puneeth Chaganti
Alexandra Ciobica
Vaibhav
ppreethi
Priyank Patel
Anders Kaseorg
YashRE42
Rohitt Vashishtha
Anders Kaseorg
Hemanth V. Alluri
Mateusz Mandera
Rishi Gupta
Kanishk Kakar
Harshit Bansal
Mayank Madan
Raymond Akornor
Yashashvi Dave
Eeshan Garg
Rafid Aslam
Marco Burstein
sumanthvrao
Abhinav Singh
Rhythm Sharma
Steve Howell
vinitS101
Greg Price
rht
konraddeka
zegervdv
Aditya Bansal
Lyla Fischer
Joshua Pan
xificurC
Max Nussenbaum
Rhea Parekh
Yago González
Cynthia Lin
Abhilash Verma
Shayan Toqraee
Armaan Ahluwalia
Shubham Dhama
Shubham Padia
Sampriti Panda
Robert Hönig
Sivakar Sithamparanathan
Michael J. Sullivan
Aman Ghebreweldi
RobbieClarken
Darshan Markandaiah
Emilio Schadt
Preston Hansen
Alyssa Wagenmaker
Tarun Kumar
Ben Reeves
neiljp (Neil Pilgrim)
sinwar
Gooca
Umair Khan
Balaji2198
ihsavru
Brock Whittaker
cPhost
Symt
picapi_
David Rosa Tamsen