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.

Sign up for GitHub

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

Support running docs builds against worktrees #2130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support running docs builds against worktrees #2130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion build_docs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,13 @@ def run_build_docs(args):
extra_name = repo_name.replace('x-pack-', '')
repo_mount = '/doc/%s-extra/%s' % (extra_name, repo_name)
else:
repo_mount = '/doc/' + repo_name
match = re.search(r'(.*)-[0-9]+\.[0-9x]+$', repo_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pretty safe way to detect "repo names" that include a -<version branch> suffix, and is unlikely to inadvertently catch a non-worktree repo name; the only repo that contains a digit right now is cloud-on-k8s.

Is this pattern for worktree names (elasticsearch, elasticsearch-7.x, elasticsearch-6.x) pretty universal and documented anywhere? I wouldn't want to have a partial solution that only works for this specific setup if it's not universal; if I understand correctly, any other worktrees that don't match this pattern would still fail, right?

Would it be possible to parse the .git file (which on my machine seems to contain something like gitdir: /absolute/path/to/elasticsearch/.git/worktrees/elasticsearch-7.x and find the repo name by looking at the path component before .git?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would match the pattern I am using for my work trees. I am not aware of any conventions/documentation for how to create these (I have a tiny bash script for personal use)

~/workspace/elasticsearch $ ls
7.0  7.1  7.10  7.11  7.12  7.13  7.14  7.15  7.16  7.17  7.2  7.3  7.4  7.5  7.6  7.7  7.8  7.9  8.0  8.1  8.2  main

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the format that we recommend in the ES development docs

$ git fetch origin 7.x
$ git checkout -b 7.x origin/7.x && git checkout master
$ git worktree add ../elasticsearch-7.x 7.x

I'll try and pick this PR up again and see if I can make it agnostic to the worktree name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh.. i had missed those. Not a big fan of that layout and the instructions needed updating anyway ... so I am taking a swing at changing the recommendation : https://github.com/elastic/elasticsearch-team/pull/882

if match is None:
repo_mount = '/doc/' + repo_name
else:
# Repo name ends with what looks like a version string (e.g. elasticsearch-7.x)
# Mount this under the unversioned project name
repo_mount = '/doc/' + match.group(1)
if repo_root not in mounted_doc_repo_roots:
if repo_name in mounted_doc_repo_names:
raise ArgError("Can't mount two repos with the same " +
Expand Down
28 changes: 24 additions & 4 deletions build_docs.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ sub _guess_opts {
my $toplevel = _find_toplevel( $index->parent );
my $remote = _pick_best_remote( $toplevel );
my $branch = _guess_branch( $toplevel );
my $repo_name = _guess_repo_name( $remote );
my $repo_name = _guess_repo_name( $remote, $toplevel );
# We couldn't find the top level so lets make a wild guess.
$toplevel = $index->parent unless $toplevel;
printf "Guessed toplevel=[%s] remote=[%s] branch=[%s] repo=[%s]\n", $toplevel, $remote, $branch, $repo_name;
Expand All @@ -169,7 +169,7 @@ sub _guess_opts {
$toplevel = _find_toplevel( $resource );
$remote = _pick_best_remote( $toplevel );
$branch = _guess_branch( $toplevel );
$repo_name = _guess_repo_name( $remote );
$repo_name = _guess_repo_name( $remote, $toplevel );
# We couldn't find the top level so lets make a wild guess.
$toplevel = $resource unless $toplevel;
$Opts->{roots}{ $repo_name } = $toplevel;
Expand All @@ -188,6 +188,23 @@ sub _find_toplevel {
chdir $docpath;
my $toplevel = eval { run qw(git rev-parse --show-toplevel) };
chdir $original_pwd;
return $toplevel if $toplevel;

my $d = $docpath;
while ($d) {
printf "Checking for .git directory (or file) in '%s'\n", $d;
my $git = $d->file(".git");
my $git_stat = $git->stat();
return $git->parent if $git_stat;
my $p = $d->parent;
if ($p eq $d) {
# Root of this filesystem
$d = undef;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just break here? Or print an error and return immediately? Is there any other way $d could become undefined in this loop?

} else {
$d = $p;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get rid of the unless $toplevel below, and then just return 0?


say "Couldn't find repo toplevel for $docpath" unless $toplevel;
return $toplevel || 0;
}
Expand Down Expand Up @@ -252,9 +269,12 @@ sub _guess_branch {
#===================================
sub _guess_repo_name {
#===================================
my ( $remote ) = @_;
my ( $remote, $toplevel ) = @_;

return 'repo' unless $remote;
if ( not $remote ) {
return 'repo' unless $toplevel;
return dir($toplevel)->basename;
}

$remote = dir( $remote )->basename;
$remote =~ s/\.git$//;
Expand Down