Jump to content

Incomplete clean-up of source code.

going

By going
in Advanced users - Development

 Share

Recommended Posts

going

going

Posted
Posted

I wish you all good health.

Using the "Armbian" build system, I noticed two bad points:

1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree.

2 - Debian kernel source package includes the git directory.

 

1) All I do I'm fixing the version kernel (KERNELBRANCH="tag:v4.14.40") and collect several variants for one Board, changing the patchset for each version, adding or removing some.

Before each new compilation I'm doing a test of the purity of the source. 

clean_repo()
{
	display_alert "clean_repo $PWD"
	#git status -s
	git reset --hard HEAD	# First, return the modified files.
	git clean -df			# Then delete all new, added files.
	#git status
}


compile_kernel()
{
	# A simple check ensures that we start with a clean slate.
	cd $SRC/cache/sources/$LINUXSOURCEDIR
	if [ "X$(git status -s)" != "X" ]; then
		clean_repo
	fi
	#
	... ...
}

 

2) Manufacturer's code Debian package:      (--exclude='./.git/') 


	if [[ $BUILD_KSRC != no ]]; then
		display_alert "Compressing sources for the linux-source package"
		tar cp --directory="$kerneldir" --exclude='./.git/' --owner=root . \
			 | pv -p -b -r -s $(du -sb "$kerneldir" --exclude=='./.git/' | cut -f1) \
			| pixz -4 > $sources_pkg_dir/usr/src/linux-source-${version}-${LINUXFAMILY}.tar.xz
		cp COPYING $sources_pkg_dir/usr/share/doc/linux-source-${version}-${LINUXFAMILY}/LICENSE
	fi

can be changed to view:    (--exclude="[.]git") 

	if [[ $BUILD_KSRC != no ]]; then
		display_alert "Compressing sources for the linux-source package"
		tar cp --directory="$kerneldir" --exclude="[.]git" --exclude="[.]git[a-z]*" --owner=root . \
			 | pv -p -b -r -s $(du -sb "$kerneldir" --exclude="[.]git" --exclude="[.]git[a-z]*" | cut -f1) \
			| pixz -4 > $sources_pkg_dir/usr/src/linux-source-${version}-${LINUXFAMILY}.tar.xz
		cp COPYING $sources_pkg_dir/usr/share/doc/linux-source-${version}-${LINUXFAMILY}/LICENSE
	fi

 

1
Link to comment
Share on other sites
chwe

chwe

Posted
Posted
5 hours ago, going said:

1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree.

got fooled by this too in the beginnings.. :P I mostly work with 'CLEAN_LEVEL = sources' when I prepare patches which create new files (https://github.com/armbian/build/blob/d9849de58fcb8fe8f6c0b01d71b61c28a6bc6f35/config/templates/config-example.conf#L6-L10).

 

your solution looks smart (but to be honest, my git-fu is to low to be sure that this works reliable). You may create a PR where the maintainers will review it this works sufficient? Would save some bandwidth when preparing such patches cause I normally need more than one attempt for a proper patch. :)

  

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted
22 minutes ago, chwe said:

'CLEAN_LEVEL = sources'

large overhead when downloading. 

git reset --hard HEAD	# First, return the modified files.
git clean -df			# Then delete all new, added files.

This code is guaranteed to work correctly. I use it for a month. "Clean slate" before you start the compilation, and no issues with the new patches.

Link to comment
Share on other sites
chwe

chwe

Posted
Posted
3 minutes ago, going said:

This code is guaranteed to work correctly. I use it for a month. "Clean slate" before you start the compilation, and no issues with the new patches.

Well, then I'm happy to see a PR from it and others probably too.. ;) 

overhead isn't that much an issue here.. thanks to fiber at home.. but for sure, if it works reliable I would still use it. :) 

Link to comment
Share on other sites
chwe

chwe

Posted
Posted

that's why I like your approach. Problem is, as long as downloading big junks of code doesn't hurt you, you don't care much. You may team up with @Igor or @zador.blood.stained how this can be implemented properly. I guess it should be here:

https://github.com/armbian/build/blob/88ea83e694d8c015c42a9ae6fb163b0e7470755e/lib/general.sh#L32-L81

But, to be honest.. I'm not 100% familiar with this part of the build-script to guide you how this should be done properly. I'm sure, they are. 

 

17 minutes ago, going said:

:D I am somewhere in the Siberian province and use 3G modem.:(

That's cool.  :) Seeing that the buildscript is used around the world and limited bandwidth helps that people think about issues which aren't 'that much an issue' for those without such limitations. I really hope we can implement it so that it gets default (or at least everyone can set it in the configs) for all armbian users. 

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted

I noticed this problem after I stopped the kernel version from moving up on a particular tag and started to change patches to remove errors.

Turned off a few patches to clear the field for observation. And saw that the big aufs patch (it adds a lot of new files) partially applied, causing even more errors, but my fixes have no effect.

Link to comment
Share on other sites
  • Igor pinned this topic
going

going

Posted
  • Author
Posted
On 8/6/2018 at 11:03 AM, going said:

1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree.

 

After many inspections, it is possible to fix this problem.

Spoiler 

Fix incomplete cleaning of the source code.

Removed duplicate code and the unit is not correctly coupled logic.
---
 lib/general.sh | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/lib/general.sh b/lib/general.sh
index 8de1a352..57ee4e64 100644
--- a/lib/general.sh
+++ b/lib/general.sh
@@ -223,52 +223,42 @@ fetch_from_repo()
 		git remote add origin $url
 	fi
 
-	local changed=false
-
 	local local_hash=$(git rev-parse @ 2>/dev/null)
 	case $ref_type in
 		branch)
 		# TODO: grep refs/heads/$name
 		local remote_hash=$(git ls-remote -h $url "$ref_name" | head -1 | cut -f1)
-		[[ -z $local_hash || $local_hash != $remote_hash ]] && changed=true
+		[[ -z $local_hash || $local_hash != $remote_hash ]] && \
+		git fetch --depth 1 origin $ref_name
 		;;
 
 		tag)
 		local remote_hash=$(git ls-remote -t $url "$ref_name" | cut -f1)
 		if [[ -z $local_hash || $local_hash != $remote_hash ]]; then
 			remote_hash=$(git ls-remote -t $url "$ref_name^{}" | cut -f1)
-			[[ -z $remote_hash || $local_hash != $remote_hash ]] && changed=true
+			[[ -z $remote_hash || $local_hash != $remote_hash ]] && \
+			git fetch --depth 1 origin tags/$ref_name
 		fi
 		;;
 
 		head)
 		local remote_hash=$(git ls-remote $url HEAD | cut -f1)
-		[[ -z $local_hash || $local_hash != $remote_hash ]] && changed=true
+		[[ -z $local_hash || $local_hash != $remote_hash ]] && \
+		git fetch --depth 1 origin HEAD
 		;;
 	esac
 
-	if [[ $changed == true ]]; then
-		# remote was updated, fetch and check out updates
-		display_alert "Fetching updates"
-		case $ref_type in
-			branch) git fetch --depth 1 origin $ref_name ;;
-			tag) git fetch --depth 1 origin tags/$ref_name ;;
-			head) git fetch --depth 1 origin HEAD ;;
-		esac
-		display_alert "Checking out"
-		git checkout -f -q FETCH_HEAD
-	elif [[ -n $(git status -uno --porcelain --ignore-submodules=all) ]]; then
-		# working directory is not clean
-		if [[ $FORCE_CHECKOUT == yes ]]; then
-			display_alert "Checking out"
-			git checkout -f -q HEAD
-		else
-			display_alert "Skipping checkout"
-		fi
-	else
-		# working directory is clean, nothing to do
-		display_alert "Up to date"
+	# Return the files that are tracked by git to the initial state.
+	display_alert "Checking out"
+	git checkout -f -q FETCH_HEAD
+	if [ "blank$(git status -s)" != "blank" ]; then
+		# Files that are not tracked by git and were added
+		# when the patch was applied must be removed.
+		display_alert "Cleaning source" "$PWD"
+		git clean -df
 	fi
+	display_alert "Up to date"
+
 	if [[ -f .gitmodules ]]; then
 		display_alert "Updating submodules" "" "ext"
 		# FML: http://stackoverflow.com/a/17692710
-- 
2.13.7

 

0001-Fix-incomplete-cleaning-of-the-source-code.patch

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted
Spoiler 

Check that to understand what it is talking.

Optional patch.
Install at the beginning to see the files added during
the previous patching and compilation.
---
 lib/main.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/main.sh b/lib/main.sh
index e2e274a1..cdc50156 100644
--- a/lib/main.sh
+++ b/lib/main.sh
@@ -275,6 +275,15 @@ for option in $(tr ',' ' ' <<< "$CLEAN_LEVEL"); do
 	[[ $option != sources ]] && cleaning "$option"
 done
 
+# Checking for purity src atf, uboot, kernel
+# -it's just information for thought.
+cur_dir=$PWD
+LIST_SRC="$SRC/cache/sources/$ATFSOURCEDIR $SRC/cache/sources/$BOOTSOURCEDIR $SRC/cache/sources/$LINUXSOURCEDIR"
+for IN in $LIST_SRC; do
+	cd $IN; display_alert "In source of $IN dirty files: " "$(git status -s | wc -l)"
+done
+cd $cur_dir
+
 # Compile u-boot if packed .deb does not exist
 if [[ ! -f $DEST/debs/${CHOSEN_UBOOT}_${REVISION}_${ARCH}.deb ]]; then
 	if [[ -n $ATFSOURCE ]]; then
-- 
2.13.7

 

0002-Check-that-to-understand-what-it-is-talking.patch

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted
5 hours ago, Igor said:

I looks OK ... let's test a bit but I guess you can sent a PR to the build script. Thanks!

Igor, sorry but the automatic translation does not allow me to understand, you must also send the patches somewhere else? Or is it just enough to put them here? I did not find a description of the patch acceptance procedure in the documentation.

I have a lot of fixes. Well tested I want to send for consideration to acceptance.

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted
2 hours ago, guidol said:

:D I am somewhere in the Siberian province and use 3G modem.:(

In fact, I'm good with the Internet and all the necessary repositories are on a nearby SSD.

There can be several reasons:

- Slow Internet and unwillingness to clean sources every time.

- Inattention, forgetfulness, typo.

- Ignorance of documentation.

In any negative scenario, the source directories must be clean before compiling.

Link to comment
Share on other sites
Igor

Igor

Posted
Posted
1 hour ago, going said:

you must also send the patches somewhere else?

 

I can add patches but since they are not signed they will retain my name. I want to avoid that whenever possible, especially when author is known. The right way is to create a pull request.

 

1 hour ago, going said:

I have a lot of fixes. 

 

Than is great to hear! Then it is even more important to learn how to do it properly. 

Here are some general instructions how to create a pull request on Github:

https://www.armbian.com/get-involved/#submit 

https://docs.armbian.com/Process_Contribute 

I am pretty sure there must be some guides in your native language if Google translate fails.

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted
13 hours ago, Igor said:

Than is great to hear! Then it is even more important to learn how to do it properly. 

Thanks. I saw the applied patches for testing.

I read your links and realized that it is customary to make changes in a separate branch and then pull request. 

Of course, first we need to discuss with the community the need to develop new functionality. I will do so in the future.

I'm currently laying out a number of patches that I think are the most important. Although it is not a tradition.

You can apply them on your own behalf, after testing, if you see them reasonable. I don't claim authorship.

Thank you for your understanding.

1
Link to comment
Share on other sites
lanefu

lanefu

Posted
Posted
7 hours ago, going said:

Igor, let's try to open the eyes of users.

Please apply these two patches.

 

 

Hey thanks for putting in a lot of effort to make the patches.  Could you submit them as a PR on https://github.com/armbian/build

 

There are a few others besides igor that can apply discretion and merge practical fixes.

Link to comment
Share on other sites
going

going

Posted
  • Author
Posted

I apologize if I seem rude.
I use automatic translation.
Sometimes the meaning can turn over.

 

And thank you.
I started working with github.
Now I understand why you don't accept the patches as a patch file

1
Link to comment
Share on other sites
  • Igor unpinned this topic
Guest
This topic is now closed to further replies.
 Share
Go to topic listing
×
×
  • Create New...

Important Information

Terms of Use - Privacy Policy - Guidelines

  I accept