Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2020, 23:59 UTC
Message-ID: <xmqqo8qrqtch.fsf@gitster.c.googlers.com>
In-Reply-To: <20200513235215.GA10618@danh.dev>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 26 quoted lines

>> >  
>> > +chmod a+w /
>> > +
>> >  # Build and test
>> >  command $switch_cmd su -m -l $CI_USER -c "
>> >  	set -ex
>> > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c "
>> >  	export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB'
>> >  	export MAKEFLAGS='$MAKEFLAGS'
>> >  	export cache_dir='$cache_dir'
>> > +	export IKNOWWHATIAMDOING=YES
>> >  	cd /usr/src/git
>> >  	test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove
>> >  	make
>> 
>> Big EWWWWWWwwww.  Do we need to do this for _all_ tests, not
>> selectively only while running t1509?  This makes me worried as a
>> test by mistake can easily corrupt the VM and invalidating the
>> tests; I know we get a fresh one every time, so there is no
>> permanent harm done by corrupting it, but having one fewer thing we
>> have to worry about is always better than having one more thing.
>
> Perhaps pass this variable all the way down from ci/lib.sh?
> Adding another variable into t1509 (except T1509_*) doesn't make it
> less risky.
> Or should we add T1509_ prefix to this env var?

I was not worried about any environment variable, but the "let's make the root directory writable by anybody during _all_ tests", when we need such a crazy permission bits on the filesystem only while running t1509 and not any other time, stood out as extremely yucky.