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

Remove the --cs/--skip arguments and add an example of negative regex #74

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

marcosps
Copy link
Collaborator

No description provided.

@marcosps marcosps requested review from vmezzela and fgyanz January 28, 2025 17:51
klpbuild/plugins/extractor.py Show resolved Hide resolved
klpbuild/klplib/ibs.py Show resolved Hide resolved
klpbuild/klplib/utils.py Show resolved Hide resolved
@marcosps
Copy link
Collaborator Author

@vmezzela done, removed the skip mention on filter_codestreams.

@vmezzela vmezzela self-requested a review January 30, 2025 12:28
Copy link
Collaborator

@vmezzela vmezzela left a comment

Choose a reason for hiding this comment

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

Thanks @marcosps !!

@fgyanz
Copy link
Collaborator

fgyanz commented Jan 31, 2025

Just one last thing. Maybe it's on my side, but before I starting digging deeper do you mind re-running the tests? They're failing on me with:
FAILED tests/test_templ.py::test_check_header_file_included - subprocess.CalledProcessError: Command 'find /home/fgonzalez/klp/data/x86_64/lib/modules/4.12.14-122.244-default -name "*.zst" -exec unzstd -f -d --quiet {} \;' returned non-zero exit status 1.

My bad, the issue was fixed in #73

@marcosps
Copy link
Collaborator Author

marcosps commented Jan 31, 2025

Just one last thing. Maybe it's on my side, but before I starting digging deeper do you mind re-running the tests? They're failing on me with: FAILED tests/test_templ.py::test_check_header_file_included - subprocess.CalledProcessError: Command 'find /home/fgonzalez/klp/data/x86_64/lib/modules/4.12.14-122.244-default -name "*.zst" -exec unzstd -f -d --quiet {} \;' returned non-zero exit status 1.

My bad, the issue was fixed in #73

Is this a reviewed-by tag? :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget to remove --cs from log command :)

klp-build.1 Show resolved Hide resolved
@@ -588,11 +589,12 @@ def create_lp_package(self, i, cs):

logging.info(f"({i}/{self.total}) {cs.name()} done")

def log(self, cs, arch):
def log(self, arch):
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW log now is able to download logs for more than one codestream (e.g. "15.6u1|15.6u2"), but it crashes if I pass "15.6u[1-2]". Up for debate but maybe it would wise to restrict it to just one codestream. I can't think of a scenario in which we would need more that one log. Ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

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

One possible approach could be to always require a mandatory argument for this command, specifying the name of the codestream whose log is needed (not necessarily in this PR)

args = []
def cs_diff(self):
"""
To compare two codestreams the filter should result in exactly two codestreams
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, I think it would be a good idea to print which codestream is used as base for the diff. I mean, if I pass --filter "15.6u[1-2]", probably u1 will be used as base, but is just a guess.
Perhaps we could do something like :

--- 15.6u1

+++ 15.6u2

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about the following?

  --- 15.5u9 net/ipv4/tcp_input.c
   
 +++ 15.6u0 net/ipv4/tcp_input.c

I've implemented this on the last update.

@marcosps
Copy link
Collaborator Author

Oops... I implemented the fixes on top of last commit... it's no a big deal, but I don't think it's a problem :)

There is an example in the manpage about how to avoid a codestream using
a negative regex.

Signed-off-by: Marcos Paulo de Souza <[email protected]>
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.

3 participants