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

Use subdir named gitea-version in make release-sources generated arti… #19153

Closed
wants to merge 1 commit into from
Closed

Use subdir named gitea-version in make release-sources generated arti… #19153

wants to merge 1 commit into from

Conversation

eleksir
Copy link

@eleksir eleksir commented Mar 20, 2022

Use tar --transform to place gitea sources within a sub-directory called gitea-${VERSION} to prevent tar bombing.

Fix #19066

⚠️ BREAKING ⚠️

Prior to this PR the gitea src tar.gz would extract to the current directory. Users requiring the previous behaviour should use tar options as appropriate.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 20, 2022
@zeripath zeripath added pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile labels Mar 20, 2022
@zeripath zeripath added this to the 1.17.0 milestone Mar 20, 2022
@zeripath
Copy link
Contributor

zeripath commented Mar 20, 2022

OK we need to double check that bsdtar has this option too.

Annoyingly bsdtar appears to call this option -s instead.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2022

OK we need to double check that bsdtar has this option too.

https://www.freebsd.org/cgi/man.cgi?query=tar&sektion=1&manpath=freebsd-release-ports
(alternative: gtar)

BSD tar doesn't support --transform, but do we really need to tar in BSD? Our pipelines are all running in Linux.

@zeripath
Copy link
Contributor

The line above this makes reference to making changes for bsdtar.

See #15256

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2022

The line above this makes reference to making changes for bsdtar.

See #15256

Hmm, I see. I was thinking that make release-sources is only used by Gitea's official pipelines (I can not imagine a case that why somebody wants to run make release-sources locally). If down-streams want to pack the source code, they should have their own package toolchains .....

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2022

Now I have another question (a little off-topic): why should release-sources support so many OS? If these OS must be supported .... since Gitea pipeline uses docker a lot to do build, maybe we could just use a latest Linux image in Makefile to provide the tar command.

@eleksir
Copy link
Author

eleksir commented Mar 20, 2022

bsd tar have option -s

     -s	pattern
	     Modify file or archive member names according to pattern.	The
	     pattern has the format /old/new/[ghHprRsS]	where old is a basic
	     regular expression, new is	the replacement	string of the matched
	     part, and the optional trailing letters modify how	the replace-
	     ment is handled.  If old is not matched, the pattern is skipped.
	     Within new, ~ is substituted with the match, \1 to	\9 with	the
	     content of	the corresponding captured group.  The optional	trail-
	     ing g specifies that matching should continue after the matched
	     part and stop on the first	unmatched pattern.  The	optional
	     trailing s	specifies that the pattern applies to the value	of
	     symbolic links.  The optional trailing p specifies	that after a
	     successful	substitution the original path name and	the new	path
	     name should be printed to standard	error.	Optional trailing H,
	     R,	or S characters	suppress substitutions for hardlink targets,
	     regular filenames,	or symlink targets, respectively.  Optional
	     trailing h, r, or s characters enable substitutions for hardlink
	     targets, regular filenames, or symlink targets, respectively.
	     The default is hrs	which applies substitutions to all names.  In
	     particular, it is never necessary to specify h, r,	or s.

But i've no bsd/mac system on my hands to test it

@silverwind
Copy link
Member

silverwind commented Mar 21, 2022

why should release-sources support so many OS

I guess it's still valuable to support macOS to be able to debug these make targets locally. We could just require those developers to install GNU versions, e.g. abort the target if non-GNU is detected. GNU versions on macOS either have a "g" prefix, or not, e.g. gtar or tar (in case user has installed it in-place).

@wxiaoguang
Copy link
Contributor

why should release-sources support so many OS

I guess it's still valuable to support macOS to be able to debug these make targets locally. We could just require those developers to install GNU versions, e.g. abort the target if non-GNU is detected. GNU versions on macOS either have a "g" prefix, or not, e.g. gtar or tar (in case user has installed it in-place).

That could be a reason .... however although I am a macOS user, I do not use non-Linux tools to debug systems for Linux, it doesn't make sense to me because finally these targets should be run in Linux. Personally I always use VM or docker for the work which should be in a Linux environment.

Maybe require those developers to install GNU versions is a better choice to simplify the Makefile.

@silverwind
Copy link
Member

Maybe require those developers to install GNU versions is a better choice to simplify the Makefile.

Still need to check for existance of gtar vs tar for example, so there's still some complexity involved. If we can support bsdtar here without much effort, I'd say we go for that now.

@wxiaoguang
Copy link
Contributor

Replaced by

@wxiaoguang wxiaoguang closed this Apr 15, 2022
@wxiaoguang wxiaoguang removed this from the 1.17.0 milestone Jun 12, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tarbomb in release src tarball file
6 participants