FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 03-22-2011, 10:55 PM
Mike Frysinger
 
Default git-2.eclass final review

On Tue, Mar 22, 2011 at 7:41 PM, Ryan Hill wrote:
> On Tue, 22 Mar 2011 19:08:53 -0400 Mike Frysinger wrote:
>> > rsync -rlpgo . "${EGIT_SOURCEDIR}"
>>
>> this means you need to have DEPEND="net-misc/rsync". *why not just use
>> `cp -pPR` instead ? *i vaguely recall rsync being slower than a
>> straight cp too ... not much point of doing a rsync when the vast
>> majority of the time (all the time?) the destination is empty.
>
> i think i remember doing some time tests using a tar pipe at one point with
> subversion.eclass that blew cp out of the water.
>
> tar -cf - . | tar -xf - -C "${EGIT_SOURCEDIR}"
>
> might be worth looking at.

ideally, the git eclass should be creating bare checkouts only in its
store dir, in which case it could use `git archive | tar` to move
things over ...
-mike
 
Old 03-22-2011, 11:42 PM
Tomáš Chvátal
 
Default git-2.eclass final review

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dne 23.3.2011 00:08, Mike Frysinger napsal(a):
> 2011/3/22 Tomáš Chvátal:
>> Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
>>>> # @BLURB: This eclass provides functions for fetch and unpack git repositories
>>>
>>> fetching/unpacking
>>
>> Yarp fixed.
>
> well, the fix broke the blurb. it has to be on one line.
> # @BLURB: foo
Oh damn :P
>
>> EGIT_BRANCH=${x:-${EGIT_BRANCH:=${EGIT_MASTER}}}
>> EGIT_COMMIT=${x:-${EGIT_COMMIT:=${EGIT_BRANCH}}}
>
> doesnt make much sense to use := ... it should be :-
Yeah it should be but still it was just copy/paste and did no harm
Altered
>
>> [[ "$#" -ne 1 ]] && die "${FUNCNAME}: requires 1 argument (path)"
>
> quoting doesnt make much sense ... -ne compares an int, not a string
>
> also, the error msg is a bit vague. it should say "... requires exactly 1 ..."
Altered
>
>> pushd "${1}" &> /dev/null
>> popd &> /dev/null
>
> do you really want to silence errors ? normally people only send
> stdout to /dev/null because of the echoed dirlist.
>
> seems to come up in every func
Yeah that does not matter that much, altered to make stderr visible again.
>
>> git submodule init || die "${FUNCNAME}: git submodule initialisation failed"
>> git submodule sync "" die "${FUNCNAME}: git submodule sync failed"
>> git submodule update || die "${FUNCNAME}: git submodule update failed"
>
> the die strings are abit redundant ... when it fails, the output will
> show "git submodule init || die", so people can easily figure out "the
> git submodule init cmd failed"
Well mostly nobody reads die messages but right here it is pointless.
>
>> die ""${EGIT_BOOTSTRAP}" is not executable."
>
> i find periods in die messages which are a single sentence to be
> noise. but maybe that's just me.
Does not matter for me either. Since using both i will just stick
withoutdot for dies everywhere
>
>> if [[ "${EGIT_COMMIT}" != "${EGIT_BRANCH}" ]]; then
>
> quoting doesnt matter here since you're using [[...]]
>
> seems to come up in a bunch of places
Yeah overquoting is my favorite thing to do during the long and boring
evenings removed
>
>> local save_sandbox_write=${SANDBOX_WRITE}
>> if [[ ! -d "${EGIT_STORE_DIR}" ]] ; then
>> ...
>> SANDBOX_WRITE=${save_sandbox_write}
>> fi
>
> might as well have the save done inside the if statement since that's
> the only place it's used
>
I actualy can't find any good reason why that var is defined at all.
Removed the code.

>> rsync -rlpgo . "${EGIT_SOURCEDIR}"
>
> this means you need to have DEPEND="net-misc/rsync". why not just use
> `cp -pPR` instead ? i vaguely recall rsync being slower than a
> straight cp too ... not much point of doing a rsync when the vast
> majority of the time (all the time?) the destination is empty.

Good question, i actualy dunno why i picked rsync back then instead of cp.
I would love to use git bare clones (damn submodules) and just use git
to move this crap around. cp -pPR should be equal so lets try with that
and see how much people whine around
>
>> git-2_initial_clone()
>> git-2_update_repo()
>
> shouldnt EGIT_REPO_URI_SELECTED be set to "" at the start ?
Why not
And found issue in die string because it said wrong variable there
>
>> for x in $(git branch |grep -v "* ${EGIT_MASTER}" |tr '
' ' '); do
>
> missing space after each pipe
Added
>
>> if [[ -n ${EGIT_BOOTSTRAP} ]] ; then
>> if [[ -f ${EGIT_BOOTSTRAP} ]]; then
>
> seems inconsistent in the whole file ... personally, i prefer the
> space before the semicolon in if statements
Yerp i go with the later mostly but sometimes when i copy part statement
i get space into it. Unified.

Thanks for all the points

Again the diff is: http://tinyurl.com/62eb88b

Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2JQgsACgkQHB6c3gNBRYf79QCfV454YtzZD+ 2m7fOvFHvriDnf
0+4AnjnCA9oCwxPyzWsl8YzAI8ihXdm1
=qdu+
-----END PGP SIGNATURE-----
 
Old 03-23-2011, 11:28 AM
James Cloos
 
Default git-2.eclass final review

>>>>> "MF" == Mike Frysinger <vapier@gentoo.org> writes:

MF> ideally, the git eclass should be creating bare checkouts only in its
MF> store dir, in which case it could use `git archive | tar` to move
MF> things over ...

Or better yet, git clone.

Most builds from vcs work best when they know that they are building
from vcs.

-JimC
--
James Cloos <cloos@jhcloos.com> OpenPGP: 1024D/ED7DAEA6
 
Old 03-23-2011, 12:01 PM
Tomáš Chvátal
 
Default git-2.eclass final review

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dne 23.3.2011 13:28, James Cloos napsal(a):
>>>>>> "MF" == Mike Frysinger <vapier@gentoo.org> writes:
>
> MF> ideally, the git eclass should be creating bare checkouts only in its
> MF> store dir, in which case it could use `git archive | tar` to move
> MF> things over ...
>
> Or better yet, git clone.
>
> Most builds from vcs work best when they know that they are building
> from vcs.
>
> -JimC
I explained multiple times already why bare checkouts are not working in
our case.
I don't get why you guys keep repeating it like in the loop.
(I also asked for some implementation where the bare would be possible
with submodules stored in distdir, yet nobody said it is possible)

So live with it.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2J7ygACgkQHB6c3gNBRYdVMACfUPxairvFms CM+gHTODS1EhTM
W0MAoKhDAnzkuOV0HUHfrBUmcY+nPli7
=WVty
-----END PGP SIGNATURE-----
 
Old 03-23-2011, 01:44 PM
James Cloos
 
Default git-2.eclass final review

>>>>> "TC" == Tomáš Chvátal <scarabeus@gentoo.org> writes:

TC> I explained multiple times already why bare checkouts are not
TC> working in our case.

Wait a minute.

Not using bare clones in DISTDIR is completely unacceptable here.

It is bad enough to have to use non-bare for repos which have
submodules. Doing so for all is b0rked.

TC> I don't get why you guys keep repeating it like in the loop.

I didn't notice that bug until you just pointed it out. Nor did I see
any of the explanations you mention above.

TC> (I also asked for some implementation where the bare would be possible
TC> with submodules stored in distdir, yet nobody said it is possible)

TC> So live with it.

I cannot. It makes the eclass useless.

I have almost 2 gigs of bare repo in distdirs/git-src.

A forced re-download of all of that is just not possible!

The existing distdir clones *MUST* continue to work.

My applogies for not having looked for this kind of breakage in the new
eclass before now. The current git eclass finally got the submodules-
vs-normal stuff worked out some time ago; the possibility of going
backwards never occurred to me.... ☹

As someone who makes heavy use of live ebuilds, someone who will be
directly and severely affected by such a change, I have to beg you
to keep the current logic for submodule-less repos.

P.S. The kind of clone used in distdir is irrelevant to the
fact that git-clone should be used to populate $S.

-JimC
--
James Cloos <cloos@jhcloos.com> OpenPGP: 1024D/ED7DAEA6
 
Old 03-23-2011, 03:29 PM
Donnie Berkholz
 
Default git-2.eclass final review

On 08:28 Wed 23 Mar , James Cloos wrote:
> >>>>> "MF" == Mike Frysinger <vapier@gentoo.org> writes:
>
> MF> ideally, the git eclass should be creating bare checkouts only in its
> MF> store dir, in which case it could use `git archive | tar` to move
> MF> things over ...
>
> Or better yet, git clone.

This could work well with --shared; even worked for me on separate
partitions.

--
Thanks,
Donnie

Donnie Berkholz
Sr. Developer, Gentoo Linux
Blog: http://dberkholz.com
 
Old 03-24-2011, 09:52 AM
James Cloos
 
Default git-2.eclass final review

>>>>> "DB" == Donnie Berkholz <dberkholz@gentoo.org> writes:

JC> Or better yet, git clone.

DB> This could work well with --shared; even worked for me on separate
DB> partitions.

Yes, I did mean git clone -l -s.

-JimC
--
James Cloos <cloos@jhcloos.com> OpenPGP: 1024D/ED7DAEA6
 
Old 03-24-2011, 11:54 AM
Donnie Berkholz
 
Default git-2.eclass final review

On 10:44 Wed 23 Mar , James Cloos wrote:
> TC> So live with it.
>
> I cannot. It makes the eclass useless.
>
> I have almost 2 gigs of bare repo in distdirs/git-src.
>
> A forced re-download of all of that is just not possible!
>
> The existing distdir clones *MUST* continue to work.
>
> My applogies for not having looked for this kind of breakage in the new
> eclass before now. The current git eclass finally got the submodules-
> vs-normal stuff worked out some time ago; the possibility of going
> backwards never occurred to me.... ☹
>
> As someone who makes heavy use of live ebuilds, someone who will be
> directly and severely affected by such a change, I have to beg you
> to keep the current logic for submodule-less repos.

I was discussing a couple of ideas on IRC with scarabeus yesterday but
haven't had a chance to look into them in more detail yet:

- Providing automatic migration from the old system to the new. This is
the simplest approach to deal with your specific problem but still
leaves separate codepaths for submodule and non-submodule repos.

- Handling submodules in bare checkouts. Perhaps we could detect whether
submodules are in use (need to find the right place/way in git), then
grab them as separate bare checkouts that would eventually be cloned
into TMPDIR by changing the repo location git looks for (again, need to
sort out how in git).

--
Thanks,
Donnie

Donnie Berkholz
Sr. Developer, Gentoo Linux
Blog: http://dberkholz.com
 
Old 03-31-2011, 04:55 AM
Jeroen Roovers
 
Default git-2.eclass final review

On Wed, 23 Mar 2011 01:42:51 +0100
Tomáš Chvátal <scarabeus@gentoo.org> wrote:

> Again the diff is: http://tinyurl.com/62eb88b

Why not attach it? What the hell does that URL lead me to?


jer
 
Old 03-31-2011, 06:32 AM
Tomáš Chvátal
 
Default git-2.eclass final review

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dne 31.3.2011 06:55, Jeroen Roovers napsal(a):
> On Wed, 23 Mar 2011 01:42:51 +0100
> Tomáš Chvátal <scarabeus@gentoo.org> wrote:
>
>> Again the diff is: http://tinyurl.com/62eb88b
>
> Why not attach it? What the hell does that URL lead me to?
>
>
> jer
>

Because our git webservice allows us to do nice colorfull diffing, but
sadly that url is by default really long...?

You know since we gentoo developers usually post tinyurled porn sites
links to -dev you just can't trust anything I post here...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2UH/sACgkQHB6c3gNBRYc7/ACfTfZBFaaWZ6ttc2Yv6J9Azet7
W/cAn2K5lB390SPGh2pSDTyiCDkalxia
=N3T6
-----END PGP SIGNATURE-----
 

Thread Tools




All times are GMT. The time now is 11:40 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org