Skip to content

make 'pkg updating' to recognize regular expression package names #1765

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

Merged
merged 4 commits into from
Jun 27, 2019
Merged

make 'pkg updating' to recognize regular expression package names #1765

merged 4 commits into from
Jun 27, 2019

Conversation

yuichiro-naito
Copy link

I added matcher function for matching regular expression package names in AFFECTS: line.
This function split AFFECTS: line into words and each words are checked if it contains regular expression characters or not.
If a word contains regular expression characters, it will be compiled and checked matches to ports origin name.

Compiling a regular expression is relatively slow operation, so I made a cache for compiled objects.

And I added special case of 'AFFECTS: all users' and 'AFFECTS: all ports users'.
The message in these cases should be shown.

This pull request solves #227193 on FreeBSD Bugzilla.

Pkg updating collects package names which is indicated by regular expressions.
@yuichiro-naito
Copy link
Author

Please review and merge this pull request.

@bapt
Copy link
Member

bapt commented Jun 18, 2019

thank you, I will review asap!

@yuichiro-naito
Copy link
Author

yuichiro-naito commented Jun 25, 2019

@bapt
Are you busy now?

Some one claims as same as #227193 in freebsd-users-jp mailing list.

I hope you have enough time to review.

@bapt
Copy link
Member

bapt commented Jun 25, 2019

yes sorry I have been busy, I will try to review before the end of the week, sorry for the delay

@bapt
Copy link
Member

bapt commented Jun 25, 2019

note that it looks globally good, (from a quick review I already did, I just want to make a more in depth review to see if I have not missed something) would it be possible for you to add some test cases?

@yuichiro-naito
Copy link
Author

OK, my test cases are followings.
I used the latest UPDATING file.

Test Case Expected Result Comment
install net/samba48 20190619 is shown includes [] expression
install lang/perl5.26 20181213 is shown includes * expression
install graphics/ilmbase 20180922 is shown comma is just after the package name
(always) 20171130 is shown all ports users message should be always shown
(always) 20131008 is shown all users message should be always shown
install mysql55-server 20150204 is shown includes () expression
install postgresql95-server 20141208 is shown includes ? expression
install print/cups-base 20110828 is shown includes {} expression

There is no test cases for '^', '$', '+' expressions.
If we test these cases, we have to add new "AFFECTS:" line to UPDATING file.

@bapt
Copy link
Member

bapt commented Jun 25, 2019

I was more saying adding test case to the testsuite to ensure we have no regression (see the tests directory, in particular frontend/updating)

@yuichiro-naito
Copy link
Author

Sorry about misunderstanding.
I will try to update test code.

Yuichiro NAITO added 3 commits June 26, 2019 11:53
In UPDATING file, simplified regular expression is used.
So we convert to POSIX 1003.2 style mentioned in re_format(4).
This makes 'print/cups' not to match 'print/cups-base'.
@yuichiro-naito
Copy link
Author

I added tests/frontend/updating.sh to test pkg updating message.

Through this test,
I found regular expression should be converted to POSIX style mentioned in re_format(4).
This is fixed in c750051.

And I think 'print/cups' package should not match to 'print/cups-base',
so I fixed to match more exactly in 827747d.

@yuichiro-naito
Copy link
Author

@bapt
Please check my test code, too.

@bapt
Copy link
Member

bapt commented Jun 26, 2019

sure thanks a lot!

@bapt bapt merged commit 292e2fb into freebsd:master Jun 27, 2019
@bapt
Copy link
Member

bapt commented Jun 27, 2019

Thanks a lot for the contribution!

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.

2 participants