The time now is Mon 09 Dec 2019, 13:02
All times are UTC - 4 |
Page 2 of 2 [19 Posts] |
Goto page: Previous 1, 2 |
Author |
Message |
s243a
Joined: 02 Sep 2014 Posts: 2199
|
Posted: Mon 26 Aug 2019, 02:21 Post subject:
|
|
Over the last week (give or take) my focus has been on improving my fork of sc0ttman's package manager (i.e. package), which is part of my woof-next fork. To me the package manger is a key component of the distribution and in many cases is also a key component of the build system. I actually plan to make pkg one of the principle build tools of my woof-next fork.
Anyway, in my fork of package there was an issue with some built-in files being re-installed. This is likely also an issue in the original version of pkg due to the fact that concurrency is used without locking the file which gives the list of installed packages. Some related posts are:
1. identifies where installed packages are filtered out from the list of dependencies.
2. identify that concurrency is used due to wait loop in the get_deps function.
3. Identify where the forking takes place in the pkg_get() function via the background call to list_deps(). Also propose some improvement to the concurancy strategy and other issues.
Anyway, the issue where built-in files are re-installed is probably fixed but I haven't tested this yet. That said the improvements that I made to the concurrency strategy has significantly sped up pkg by ensuring that that the list of installed packages is only produced once by only creating a new list of installed files if it doesn't exist:
Code: |
[ ! -f $TMPDIR/installed_pkgs ] && list_all_installed_pkgs
|
woof-code/rootfs-packages/PKG/usr/sbin/pkg#L6139
We also make sure that the list is completely created before using it. This is accomplished with the following wait loop:
Code: |
while [ ! -f "${TMPDIR}/installed_pkgs_finished" ];do #TODO: s243a: consider adding readlink here
echo -n '.'
sleep 0.1
done
|
woof-code/rootfs-packages/PKG/usr/sbin/pkg#L6182
The finished flag is simply a symlink to the list of installed packages.
Code: |
list_all_installed_pkgs(){
...
ln -s $TMPDIR/installed_pkgs $TMPDIR/installed_pkgs_finished
|
/woof-code/rootfs-packages/PKG/usr/sbin/pkg#L6122
The list of installed packages is only cleaned up once all instances of list_deps() exit:
Code: |
list_deps_count=0 #The number of concurrent instances of list_deps()
|
rootfs-packages/PKG/usr/sbin/pkg#L28
Code: |
6132 list_deps(){
6133 list_deps_count=$(( list_deps_count + 1 ))
...
6284 if [ $list_deps_count -eq 1 ]; then
6285 rm "${TMPDIR}/installed_pkg"* 2>/dev/null
6286 fi
|
rootfs-packages/PKG/usr/sbin/pkg#L6284
** considering moving this cleanup to the end of pkg.
*** My last comment I erroneously had the "-eq 1" and also forgot a "$" sign in the variable name of the if statement.
I also gave each instance of list_deps() a unique id so that they can use unique temporary files. This keeps me from having to worry about locking the temporary files.
/woof-code/rootfs-packages/PKG/usr/sbin/pkg#L29
Code: |
6132 list_deps(){
...
6152 dep_id=$((dep_id +1))
6153 local ldepID=dep_id
|
/woof-code/rootfs-packages/PKG/usr/sbin/pkg#L6152
Code: |
6132 list_deps(){
...
6180 echo "$deps" | tr ' ' '\n' | tr ',' '\n' | sort -u > $TMPDIR/all_deps_${ldepID}_0
...
6187 comm -23 ${TMPDIR}/all_deps_${ldepID}_0 ${TMPDIR}/installed_pkgs | sort -u | grep -v ^$ > ${TMPDIR}/all_deps_${ldepID}_
|
/woof-code/rootfs-packages/PKG/usr/sbin/pkg#L6180
Anyway, ${ldepID} is used a fair number of places in the code. The above changes were made in commit 8760d9b8655c5845bba889026c57a0f085566674. The changes are a bit obscured because I added a space between the "semicolon" and the "then keyword" for each instance where there was no space. I thought that this might make the syntax highlighting parse better on pastebin. It didn't.
On a final note I made some logic changes surrounding the HIDE_BUILTINS environmental variable (see post) in both the function list_all_installed_pkgs() and also the function list_deps().
Edit: When writing this post, I noticed that some changes weren't made that I had intended to made. These remaining changes I pushed in commit b5d1483424615add2ab4715171dbaf996137ee75
_________________ Find me on minds and on pearltrees.
|
Back to top
|
|
 |
s243a
Joined: 02 Sep 2014 Posts: 2199
|
Posted: Tue 27 Aug 2019, 21:03 Post subject:
|
|
The above modifications didn't fix the issue of libstdc++-5.0.6 being installed as a firefox-esr dependency when I already had libstdc++6 installed on my systems.
Code: |
5984 list_deps(){
...
6064 for subdep in `sort -u $deps_list_file | grep -v ^$`
6065 do
...
6234 subdeps_entry="`get_deps_entry "$subdep"`"
6085 subdeps="${subdeps_entry/+/}" # remove first + at start of line
6086 subdeps="${subdeps//,+/ }" # remove others .. DEPS will now be just 'dep1 dep2 dep3'
6087 subdeps="${subdeps// / }" # remove double spaces
6089 if [ "$subdeps" != '' ] && [ "$subdeps" != ' ' ];then
6290 # remove everything after the + (if the + is followed by alphanumeric chars), then add to tmp files
6291 echo "$subdep" >> ${TMPDIR}DEP_DONE
6293 # create the next deps list file to parse, containing the deps of this $subdep
6294 subdeps_list="`echo "${subdeps}" | tr ' ' '\n' | grep -v ^$ | sed -e 's/++/+++/g' -e 's/+[a-z0-9].*//g' | sort -u`"
6295 echo "$subdeps_list" >> $next_deps_list_file
6296 fi
6297 done
|
https://gitlab.com/sc0ttj/Pkg/blob/master/usr/sbin/pkg#L6094
Here is some output:
Code: |
+ subdeps_entry=+libc6,+libgcc1,+libstdc++6
+ subdeps=libc6,+libgcc1,+libstdc++6
+ subdeps='libc6 libgcc1 libstdc++6'
+ subdeps='libc6 libgcc1 libstdc++6'
+ '[' 'libc6 libgcc1 libstdc++6' '!=' '' ']'
+ '[' 'libc6 libgcc1 libstdc++6' '!=' ' ' ']'
+ echo libvpx4
++ echo 'libc6 libgcc1 libstdc++6'
++ tr ' ' '\n'
++ grep -v '^$'
++ sed -e s/++/+++/g -e 's/+[a-z0-9].*//g'
++ sort -u
+ subdeps_list='libc6
libgcc1
libstdc++'
+ echo 'libc6
libgcc1
libstdc++'
|
The sed statment seems proglematic as it converts libstdc++6 to libstdc++. While some linux distros might only allow one version of libstdc++ debian/devaun is set up so on could have coexisting versions of liberalies with different major versions.
I will think about how I want to modify this code.
_________________ Find me on minds and on pearltrees.
|
Back to top
|
|
 |
s243a
Joined: 02 Sep 2014 Posts: 2199
|
Posted: Wed 28 Aug 2019, 08:01 Post subject:
|
|
s243a wrote: | The above modifications didn't fix the issue of libstdc++-5.0.6 being installed as a firefox-esr dependency when I already had libstdc++6 installed on my systems.
Code: |
5984 list_deps(){
...
6064 for subdep in `sort -u $deps_list_file | grep -v ^$`
6065 do
...
6234 subdeps_entry="`get_deps_entry "$subdep"`"
6085 subdeps="${subdeps_entry/+/}" # remove first + at start of line
6086 subdeps="${subdeps//,+/ }" # remove others .. DEPS will now be just 'dep1 dep2 dep3'
6087 subdeps="${subdeps// / }" # remove double spaces
6089 if [ "$subdeps" != '' ] && [ "$subdeps" != ' ' ];then
6290 # remove everything after the + (if the + is followed by alphanumeric chars), then add to tmp files
6291 echo "$subdep" >> ${TMPDIR}DEP_DONE
6293 # create the next deps list file to parse, containing the deps of this $subdep
6294 subdeps_list="`echo "${subdeps}" | tr ' ' '\n' | grep -v ^$ | sed -e 's/++/+++/g' -e 's/+[a-z0-9].*//g' | sort -u`"
6295 echo "$subdeps_list" >> $next_deps_list_file
6296 fi
6297 done
|
https://gitlab.com/sc0ttj/Pkg/blob/master/usr/sbin/pkg#L6094
Here is some output:
Code: |
+ subdeps_entry=+libc6,+libgcc1,+libstdc++6
+ subdeps=libc6,+libgcc1,+libstdc++6
+ subdeps='libc6 libgcc1 libstdc++6'
+ subdeps='libc6 libgcc1 libstdc++6'
+ '[' 'libc6 libgcc1 libstdc++6' '!=' '' ']'
+ '[' 'libc6 libgcc1 libstdc++6' '!=' ' ' ']'
+ echo libvpx4
++ echo 'libc6 libgcc1 libstdc++6'
++ tr ' ' '\n'
++ grep -v '^$'
++ sed -e s/++/+++/g -e 's/+[a-z0-9].*//g'
++ sort -u
+ subdeps_list='libc6
libgcc1
libstdc++'
+ echo 'libc6
libgcc1
libstdc++'
|
The sed statment seems proglematic as it converts libstdc++6 to libstdc++. While some linux distros might only allow one version of libstdc++ debian/devaun is set up so on could have coexisting versions of liberalies with different major versions.
I will think about how I want to modify this code. |
My idea is to first check if the package exsists in the repo database before filtering out everything after the "+". Here is some draft code (will edit post later):
Code: |
subdeps_entry="`get_deps_entry "$subdep"`"
subdeps="${subdeps_entry/+/}" # remove first + at start of line
subdeps="${subdeps//,+/ }" # remove others .. DEPS will now be just 'dep1 dep2 dep3'
subdeps="${subdeps// / }" # remove double spaces
if [ "$subdeps" != '' ] && [ "$subdeps" != ' ' ]; then
# remove everything after the + (if the + is followed by alphanumeric chars), then add to tmp files
echo "$subdep" >> ${TMPDIR}/DEP_DONE
subdep_arry=()
while read subdep2; do
if [ ! -z "$(grep -m1 "|$subdep2|" "${ALL_REPO_DB_PATHS[@]}")" ]; then
subdep_arry+=("$subdep2")
else
subdep_arry+=$(echo subdep2 | sed -e 's/++/+++/g' -e 's/+[a-z0-9].*//g')
fi
done < <( `echo "${subdeps}" | tr ' ' '\n' | grep -v ^$` )
echo "$subdeps_list" >> $next_deps_list_file
fi
|
However, I think I should also consider priority (pacakges in the distro compat repo should take precidence. Also note that I'm using some bash syntax. I'll explain why later. This is a deviation from sc0tmann's version. Sc0ttman's version is comptable with ash.
_________________ Find me on minds and on pearltrees.
|
Back to top
|
|
 |
sc0ttman

Joined: 16 Sep 2009 Posts: 2772 Location: UK
|
Posted: Thu 05 Sep 2019, 10:15 Post subject:
|
|
I'm interested to get a lot of these fixes into Pkg ... But IMHO, the "proper" way to do it is a pull down my git repo from gitlab.com/sc0ttj/Pkg, make a new branch called "bugfixes-s243a", then add your changes in there, commit, then push to branch back .. that way we can review/comment on each change in the diff, discuss/fix/improve/revert any changes as needed, etc, etc ...
EDIT: And I am totally opposed to using Bash (instead of Ash), so I'd only accept PRs that use Ash, with no arrays.. You've just done some good speed improvements with dep resolution I've been meaning to finally figure out (nice work!), but then you make it slower again with Bash...
Also, from a previous post:
Code: | # sort and clean the search results
LANG=C cat $TMPDIR/pkglist | sort --field-separator='-' -k1,1df -k2gr -k3gr -k4gr | uniq > $TMPDIR/pkglist1 |
I use this somewhere, which busybox sort cant do .. But you can (apparently) just change --field-separator to -t..
From busybox sort -h:
Code: | -t CHAR Field separator |
_________________ Pkg, mdsh, Woofy, Akita, VLC-GTK, Search
|
Back to top
|
|
 |
|
Page 2 of 2 [19 Posts] |
Goto page: Previous 1, 2 |
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum You cannot attach files in this forum You can download files in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|