Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binbash #17

Closed
wants to merge 72 commits into from
Closed

Binbash #17

wants to merge 72 commits into from

Conversation

MarceloSpessoto
Copy link
Owner

No description provided.

uwla and others added 30 commits November 24, 2023 11:19
Reestructuring the tests folder by moving unit tests to tests/unit  will
make KW more organized and prepared for the incoming integration tests.

This commit also adds the flag `--unit` to `run_tests`, which limits the
scope of the tests to unit test. This may not seem like a big  deal  yet
becayse there are only unit tests, but it  will  be  relevant  once  the
integration tests are incorporated.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: uwla <[email protected]>
This commit only add {} around variables in a string concatenation.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Enable verbose for debug.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The `config --show` command now prints a header specifying the source of
variables along with them.

This requires the test to adapt to the new output: ignore  the  keywords
GLOBAL/LOCAL in the output and expect the default scope to be empty.

A note about the new default value  for  scope:  If  scope  defaults  to
local, the configuration output would  only  show  local  variables.  If
defaults to global, it would only show global variables. But if defaults
to empty, we can know the user has not specified an scope and  therefore
we should show both global and local configs.  That's  why  its  default
value is now an empty string.

Closes: kworkflow#894

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: uwla <[email protected]>
The name of checkpatch_wrapper.sh didn't fit with the project standard,
so rename checkpatch_wrapper.sh to codestyle.sh, and rename
checkpatch_wrapper_test.sh to codestyle_test.sh.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]>
Co-authored-by: João Paulo Pereira <[email protected]>
The checkpatch_wrapper script was out of date in terms of code style,
so modernize checkpatch_wrapper.sh to follow the current code standard.
Changes include:
- Use parse function to parse arguments, so now arguments
work in any order
- Update man page to include verbose option
- Update autocomplete to include help and verbose options

Issue kworkflow#935

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]>
Co-authored-by: João Paulo Pereira da Silva <[email protected]>
Remove duplicated config entries in deploy config list.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: uwla <[email protected]>
While packaging kw for Debian, we got multiple warnings like this:

 kworkflow: bad-whatis-entry [usr/share/man/man1/kw-build.1.gz]

This commit introduce the whatis for each kw feature.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
With this new flag, the user can install KW without  creating man pages,
which takes time due to sphinx.  This  drastically  reduce  installation
time and is specially useful in cases where the  manual  pages  are  not
needed, such as local development and continuous integration.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: uwla <[email protected]>
The 'Registered/Unregistered Mailing Lists' screen has some missing
mailing lists, due to the pagination of lore.kernel.org response.

For example, the mailing list 'lvm-devel' doesn't appear in patch-hub
because it is in the second page of lore.kernel.org.

So, add pagination factor to display all the available mailing lists.

Closes: kworkflow#901

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]>
Co-authored-by: Joao Paulo Pereira da Silva <[email protected]>
The flag `--force` should never prompt during installation, but it does
prompt in ArchLinux. Adding the flag `--noconfirm` to pacman's cmd fixes
this issue.

Closes kworkflow#983

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: uwla <[email protected]>
The `kw_config_loader` module would not read the option in the last line
of a file if the line is missing the newline character '\n'.

While it is a good practice to always add the newline character in  text
files, KW should still work in cases where it is missing, perhaps due to
a mistake. Indeed, `kw init` creates config files some of which  do  not
have the newline character in the last line (such as build.config) which
is evidence that mistakes may happen, but even in such scenarios we want
KW to function properly.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: uwla <[email protected]>
Add the following short flags:

* -C for --skip-checks
* -D for --skip-docs
* -d for --docs
* -f for --force
* -r for --completely-remove
* -t for --enable-tracing
* -v for --verbose

Usually short lowercase options  enable  some  behavior.  Thus,  we  use
uppercase to sign we disable  the  behavior:  the  flag  -D  to  disable
creation of documentation as man pages,  and  the  flag  -C  to  disable
dependencies checking.

Closes kworkflow#993.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Test kw version on ArchLinux, Debian and Fedora using Podman.

Also, change GitHub Unit Test Workflow to only run unit tests and ignore
the integration tests.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Co-authored-by: Fernando Henrique <[email protected]>
Co-authored-by: Gustavo Ueti Fukunaga <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
We mount a directory with sample config files  in  the  container,  then
`run kw config --show <config>` and compare the result with the expected
output.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Co-authored-by: Fernando Henrique <[email protected]>
Co-authored-by: Gustavo Ueti Fukunaga <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Test device info to check distribution on Archlinux, Debian and Fedora
using podman.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Co-authored-by: Fernando Henrique <[email protected]>
Co-authored-by: Gustavo Ueti Fukunaga <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide documentation on integration tests, such as the dependencies
required to run them.

Signed-off-by: uwla <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The run_tests script used a `strip_path` function which would store  the
basename of all files in the global variable  TESTS.  When  running  the
tests, we would use the find command to get the path of a file  matching
the basename of the current tests, then would check if that file  exists
and run the tests.

The problem with this approach is that it assumes all tests have  unique
basename. This is no longer the case: we now have  the  `config_test.sh`
integration test in addition to the `config_test.sh` unit test, and  the
same for device test. The old approach caused errors when attempting  to
run integration and unit tests with the same file

Running `./run_tests.sh --integration test device` would work.
Running `./run_tests.sh --unit test device` would work.
Running `./run_tests.sh test device` would fail.

To prevent this, this commit replaces the `strip_path` function with the
`set_tests` function, which stores the path of the test instead  of  its
basename. When running the tests, we use the `basename` command  on  the
test path to print out the basename of the  current  test,  rather  than
getting the path of the test from the basename of the test.

Also, the new approach uses regex to match specified tests when  running
`./run_test.sh test <test1> <test2>`. That is because the  find  command
cannot handle complex regex, only wildcards, so we use grep's  to  match
multiple test files via the provided regex.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The device module depends on `lscpi` and `ps`, which are  available  via
the packages `pciutils` and `procps`. Normally these  dependencies  will
already be installed by default, but we cannot assume they are,  so  add
them to the dependency lists.

Closes kworkflow#1000

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
To enable parallel processing of the patches kw used the slower option,
/tmp/ directory, instead of /dev/shm/ directory.

In this case, to speed up the processing, change the directory to
/dev/shm. Also add unit test for the create_shared_memory_dir function.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The unit tests `test_pre_process_xml_result` and
`test_process_patchsets` had personal information embedded in sampled
data. In this context, substitute the personal information in the
samples by anonymous information based on famous musicians.

Note: I took the opportunity to revise both unit tests, specially the
`test_process_patchsets` one. Before, the `is_introduction_patch` and
`extract_metadata_from_patch_title` functions were not mocked. Not
mocking them made the unit test highly coupled with their
implementation.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Listing patches off any mailing list could result in duplicated patches
being displayed unnecessarily.

In this sense, solve the problem by checking if the patch already has
been processed.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Bookmarking patches didn't show any information about where the patchset
was going to be downloaded.

To solve this problem, add a checkbox message when the patchset is
bookmarked or removed so the user can be sure where to find it or
that it was removed.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
If the temporary directory already existed, it would not be cleaned up.
This could cause issues if multiples deploys were made without rebooting
the target. In this context, the code now removes the temporary
directory and creates it again.

Co-authored-by: uwla <[email protected]>
Signed-off-by: João Gabriel Josephik <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
When running on repo-mode, KW  did  not  show  up  the  correct  version
because it was simply running `cat` on a  file.  This  works  in  normal
installations because the `kw` installed version is copied to  a  static
file during the installation. In repo-mode such information is  dynamic,
not static. In this case, it is necessary to run other commands.

Also, call `kworkflow_function` in `setup.sh` to avoid code duplication.
Before, `setup.sh` would run the same commands as in `src/help.sh`.

Furthermore, add a unit test for kw version on repo mode.

Closes kworkflow#986

Signed-off-by: uwla <[email protected]>
Co-authored-by: João Gabriel Josephik <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Add unit tests for kw version under normal installation.

Also, the repo-mode message is now sent to stderr. That way, it  can  be
easily ignored with a simple  redirection  to  /dev/null.  This  can  be
useful in future integration  tests  which  may  run  kw  in  repo-mode.
Otherwise, the output would always be messed up by the repo-mode message
and have to be filtered out in every test.

Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
In fedora distro, when installing kw with setup.sh, it is always
requested to install the package procps, even when it's already
installed.

The problem is that the command `rpm -q procps` seems to specifically
not recognize that this package is already installed. However, with `rpm
-q procps-ng` (which is the same exact package) it works fine.

So, change the dependency to 'procps-ng' instead of 'procps' to fix this
problem.

Fixes: kworkflow#1010

Signed-off-by: João Paulo Pereira da Silva <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
When manually configuring kw with `kw config`, it does not accept
configurations with space separation, even with single ou double quotes
put.

For example, the command
`kw config notification.sound_alert_command 'paplay berimbau.wav'`

will result in the wrong configuration
`notification.sound_alert_command=paplay`

Because of that, change `kw config` to receive the value as all
characters after the first space following configuration name,
instead of just the first word.

Fixes: kworkflow#1008

Signed-off-by: João Paulo Pereira da Silva <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide instructions for setting up podman to run in rootless mode.

Also, fix a codeblock in `tests.rst` which was not  correctly  formatted
and was displayed as inline instead of multiline.

Signed-off-by: uwla <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
rodrigosiqueira and others added 28 commits February 12, 2024 14:53
When using the --set-default option inside a kw env, it destroys the
symbolic link. This commit addresses this issue by adding the
--follow-symlinks parameter for the sed command.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
When removing a remote inside an env, the symbolic link is replaced by a
file, which is not the expected behavior when dealing with kw env. This
commit fixes this issue by adding the --follow-symlinks parameter to the
sed command inside remove_remote.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
If users try to rename a remote reference inside a kw env, the remote
feature destroys the original link. This commit fixes this issue by
adding the --follow-symlink parameter to the sed command.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
…'Details and Actions' bug

[What]

With the implementation of 'Search String in Lore', the function
`show_latest_patchsets_from_mailing_list` accepts an argument for
additional filters to be passed in the Lore request. This argument is
passed via the `screen_sequence` data structure used to pass information
between the screens that compose patch-hub UI. However, because in the
screen 'Details and Actions' the
`screen_sequence['SHOW_SCREEN_PARAMETER']` contains data about a patchset
(not an additional filter) and because this value isn't reset when
returning from this screen, `show_latest_patchsets_from_mailing_list`
makes an invalid request that causes the fetch to fail, when it
shouldn't.

In this context, make the screen 'Details and Actions' reset the value
of `screen_sequence['SHOW_SCREEN_PARAMETER']` when returning and adapt
other parts to not also break the 'Search String in Lore' feature.

[Why]

Undoubtedly, users will go back and forth between the listing of latest
patchsets and a patchset details and actions, so this bug is a major
flaw in patch-hub.

[How]

The fix goes by simply resetting the value of
`screen_sequence['SHOW_SCREEN_PARAMETER']` in the function
`show_patchset_details_and_actions` when the user hits the button
'Return'. However, because 'Search String in Lore' also uses this value
to work, just this reset breaks this feature. So, besides the reset,
we add a global variable to patch-hub UI declared in `patch_hub_core.sh`
to pass additional filters instead of using
`screen_sequence['SHOW_SCREEN_PARAMETER']` for this.

Closes: kworkflow#1028

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
…atus`

The function `get_patchset_bookmark_status` is used by patch-hub UI to
determine if given patchset is already in the Lore bookmarked database.
However, because the function uses `grep` to check if the patchset
message ID is present in the file that represents the bookmark database,
an empty message ID returns 1, which means true to being in the
database.

To fix this, check if the message ID passed as argument is empty and
return 22 EINVAL in case it is.

Note: Took the opportunity to rename the argument name from
`patchset_url` to `message_id`, as it is more precise.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
When fetching patches from Lore, patch-hub composes a query URL that
contain base filters along with additional filters. The base filters
basically aims to filter out any message from the given mailing list
that is not a patch. However, this query filter simply filters out
messages that have the string 'Re:' in the subject.

In this context, make this query filter more robust by enforcing that
the subject mustn't contain the string 're:', while also containing the
strings 'patch' or 'rfc' (all case insensitive). The query filter is
also constructed as an isolated logical expression to not conflict with
potential added filters.

Note: Took the opportunity to anonymize a test information.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Running `kw remote -h` outputs the short help of the `kw remote`
feature. In this short help, there is a typo of the word 'remote', so
fix the typo.

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Recently, we got some user feedback that they wish to know the env path
to enable them to use other kernel custom commands. This commit
addresses this issue by improving the kw env --list to also show the env
path.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The Zsh completions for the `kw remote` feature are broken, because:

  1. The `--list`, `--add`, `--rename`, and `remove` aren't mutually
     exclusive, as they should.
  2. The `--add` and `--rename` options require 2 arguments, but the
     completions assume they don't need any and suggest other options.
  3. The `--remove` option requires 1 argument, but the completions
     assume it doesn't need any and suggest other options.
  4. The `--set-default|-s` option followed by `--add` shouldn't accept
     arguments, yet it does.

In this sense, fix problems 1 to 3 by declaring `--add`, `--rename`, and
`--remove` as completions to `_arguments` instead of `_describe`, and
fix 4 by altering the completion of `--set-default|-s`, if `--add` has
been completed.

Also, to further improve the readability of the code, add names to the
arguments of options that need them (see the option `--add`, for
reference). These are not obligatory but are a way of documenting the
arguments.

Signed-off-by: David Tadokoro <[email protected]>
…ecution

This adjustment, including proper handling of options, container names, and the
command to execute with special consideration for the '/bin/sh -c' option, promotes
a more reliable and secure execution process. The commit aims to enhance the overall
functionality and robustness of the container_exec function when dealing with various
command scenarios.

Closes: kworkflow#1040

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Aquila Macedo <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Completely remove kw vars functionality as this feature has been
deprecated by the kw config --show functionality.

This commit does the following:
* change documentation to recommend kw config --show instead of kw vars
* remove kw vars documentation and base code
* remove show_variables_main function used internally by kw vars and its
  tests

Closes: kworkflow#1026

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
This commit exclude 'Distribution base' field from filtering due to the
clear indication that it won't yield a match during processing.

Closes: kworkflow#1048

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Aquila Macedo <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
This commit creates a workflow in GitHub Actions for integration tests

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Aquila Macedo <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
GitHub actions using Node.js 16 are being deprecated.
This also comes up as a warning in the workflow run logs.
This commit updates the checkout actions version to
v4 which uses Node.js 20

Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Sahil Sagwekar <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The `-s` option doesn't accept "=" or <space> between it and the value,
unlike its long form `--set-default`.

This commit fixes the documentation to reflect the above points.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Sahil Sagwekar <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
If the kw report is called without any arguments or with --all flag and
the statistics and pomodoro data is not present then ideally we should
get two separate errors. But the option_value['ERROR'] value is getting
overwritten. So we get two lines displaying the no data for pomodoro
error.  This commit adds an if clause for this condition and modifies
the tests accordingly.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Sahil Sagwekar <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The command clear-cache clears test cached, either integration or unit,
or both.

Currently, this new option only clears cached container images used in
integration tests, but in the future it could clear unit tests cache if
we need to do so.

Fixes kworkflow#1017

Signed-off-by: uwla <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The `kw patch-hub` feature depends heavily on correctly attaining
a patch metadata. This includes the patch version, the total number of
patches in its series, and its title stripped from the patch tag
(strings in the fashion of `[PATCH vX YY/ZZ]`, and the like).

However, the version and the patch tag are obtained by parsing the patch
title in not the most reliable way. The case is worst when getting the
total number of patches in the series, as this triggers chained network
requests that can range around one hundred in extreme cases, resulting
in unsatisfactory (and unpredictable) performance.

To make the attainment of patch metadata more robust, do the following:

  1. Extract methods to get each type of metadata (this was all mixed
     inside the function `extract_metadata_from_patch_title`);
  2. Consider more cases when parsing the title for metadata;
  3. Remove any network logic when obtaining the total number of
     patches.

Note: This commit also adds a function to get the number/index of the
patch in the series metadata, which is not being used by `kw patch-hub`
at the moment but will be in future commits.

Helps: kworkflow#911

Signed-off-by: David Tadokoro <[email protected]>
Co-authored-by: André Gustavo <[email protected]>
Co-authored-by: Guilherme Moreno <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
[What]

Revise the logic in processing representative patches for listing
patchsets. When listing patchsets (a set of related patches designed to
form a more significant change), a patch is elected to represent it
(either a cover letter or the actual first patch in the series), and is
called the representative.

[Why]

Correctly defining a patch as a representative or not is vital for the
`kw patch-hub` feature. Having false negatives (representative patches
that are "invisible" to the feature) shouldn't happen and also impact
the performance, as deeper fetches for data are needed for the same
number of patchsets. Currently, in some lists like git, `kw patch-hub`
sometimes misses 80% of the time, which is unacceptable.

[How]

The heuristics to determine a patch as representative is flawed because
it solely depends on the pattern of message IDs. A consistent pattern
doesn't seem to exist across the many lists that can be reliably used to
determine this information (actually, message ID patterns aren't even
consistent in the same list).

At the moment, from fetching an Atom feed with a list of patches to
having a list of representative patches sorted from the newest to the
oldest is done by:

  1. Pre-processing the Atom feed to obtain only the needed attributes
     of each patch;
  2. Traversing the pre-processed list of patches once and deciding if
     a patch is representative based on its message ID.

We chang step 1 to include more attributes and replace step 2 with two
steps to overhaul this processing and make it more robust and reliable.

After pre-processing the Atom feed, we traverse the list of patches,
marking everyone in a hashtable and storing their processed versions on
an array. Then, we traverse the list of patches again, but their
processed versions, and use a combination of heuristics based on the
patch metadata and its relation with its in-reply-to message (if it has
one). These heuristics are inspired by the source code of the b4
project, which can be found at https://github.com/mricon/b4.

Note: Many files had to be updated due to ripple effects in the change
of variable names, and the like. Also, much of the size of this commit
is due to testing code and mocked samples for them.

Closes: kworkflow#900

Signed-off-by: David Tadokoro <[email protected]>
Co-authored-by: André Gustavo <[email protected]>
Co-authored-by: Guilherme Moreno <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
…tative

When electing representative patches in the `kw patch-hub` feature,
which is vital to correctly listing patchsets, there is a border case
that is not consider: when the possible representative patch has
a in-reply-to message that has not yet been processed. In this case, the
logic always determine that the patch is representative, which isn't
true every time.

To mitigate this, make a request for the raw version of the in-reply-to
message that hasn't been processed, extract the necessary information,
and determine if the original patch is representive as before.

This implementation adds a network requisition that can incur in the
same problem `kw patch-hub` had of many chained requisitions. However,
as the number of requests is predictable (one per patch, in the extreme
case), when we add parallelization to the processing, this will be
solved.

Signed-off-by: David Tadokoro <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The processing of patches for listing patchsets has two major stages
after fetching the Atom feed with the patches and pre processing it. The
first major stage consists in traversing the pre processed list of
patches sequentially, processing each one of them individually, and
marking everyone in a hashtable. This stage is represented by the
`process_individual_patches`.

As we can leverage parallel computing in this case, parallelize the
function by issuing a dedicated thread for handling each patch. This
takes care of much of the processing done in
`process_individual_patches` resulting in a gain of performance.

Helps: kworkflow#911

Signed-off-by: David Tadokoro <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The processing of patches for listing patchsets has two major stages
after fetching the Atom feed with the patches and pre processing it. The
second major stage consists in traversing the processed list of patches
sequentially and determining if they are or not representative. This
stage is represented by the `process_representative_patches`.

As we can leverage parallel computing in this case, parallelize the
function by issuing a dedicated thread for handling each patch. This
takes care of much of the processing done in
`process_representative_patches` resulting in a gain of performance,
specially because it solves the problem of possible many chained network
requests.

Helps: kworkflow#911

Signed-off-by: David Tadokoro <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
When viewing both latest patchsets from a mailing list and the
bookmarked patchsets, the display of each item in the listing can be
improved. First, the message title has no size limit and overflows the
screen breaking the formatting of Dialog for long titles. Also, as the
the received time in Lore servers of the patch and the author name are
available, we can display these infos for the latest patchsets.

In this context, make the display of the patchset message title limited
to 60 chars, and display the received time and author name infos in the
listing of latest patchsets from a given mailing list.

Signed-off-by: David Tadokoro <[email protected]>
Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
As part of the kernel workflow, it is common for a developer to have
multiple hard disks with different distros for checking the kernel
behavior in those scenarios. When the developer swaps the disk but the
IP is still the same for the target machine, it is expected to hit the
remote host identification issue. This commit alleviates this problem by
detecting and taking action based on this type of failure.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The original databases function had some limitations, as that
the select function couldn't handle a where clause to select an
specific data and the where clause used in the remove function
could only handle equality comparassions.
In this sense, add a function that generate where clause with
multiple relational operations and update the `select_from` and
`remove_from` functions to use this new implementation.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The database functions had no implementation for when we needed to
update one or multiples attributes of an entry in the database without
replacing it. In this case, add a new function update_into to make it
possible.

Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: JGBSouza <[email protected]>
Signed-off-by: David Tadokoro <[email protected]>
The variable `KW_SOUND_DIR` was being setted to
`'/usr/share/sounds/kw'` twice

Reviewed-by: Rodrigo Siqueira <[email protected]>
Co-authored-by: Lais Nuto <[email protected]>
Signed-off-by: OJarrisonn (Jorge Harrisonn) <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
The usual "#!/bin/bash" may not work for some linux distributions.
Replace it with "#!/usr/bin/env bash"

kworkflow, being a bash-written project, requires the use of shebangs to define the interpreter environment where kw is run. Nowadays, the scripts contain the shebang "#!/bin/bash", which works normally for most of the distros.
There are, however, some exceptions, such as NixOS, where there is no /bin/bash (bash is located in /nix/store). Using workflow on these types of distros may not be
smooth, since the user is required to run kw commands preceded by bash or manually modify the code.

Closes: kworkflow#1089

Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
setup_for_symbolic_link_test

[[ -L "$symlink" ]]
assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [shellcheck] reported by reviewdog 🐶
This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten. SC2319

setup_for_symbolic_link_test

[[ -L "$symlink" ]]
assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [shellcheck] reported by reviewdog 🐶
This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten. SC2319

setup_for_symbolic_link_test

[[ -L "$symlink" ]]
assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [shellcheck] reported by reviewdog 🐶
This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten. SC2319

setup_for_symbolic_link_test

[[ -L "$symlink" ]]
assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [shellcheck] reported by reviewdog 🐶
This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten. SC2319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.