Bug 134804 - [GTK] Do not include files that are not in git in the tarball
Summary: [GTK] Do not include files that are not in git in the tarball
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2014-07-10 09:56 PDT by Carlos Garcia Campos
Modified: 2014-07-31 01:41 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.62 KB, patch)
2014-07-10 10:00 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (3.33 KB, patch)
2014-07-11 02:33 PDT, Carlos Garcia Campos
pnormand: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2014-07-10 09:56:35 PDT
With the current approach of listing the source directory and add all files except the ones excluded by the manifest, we might end up with undesired files in the tarball, like new files of work in progress patches, or files you worked on once and forgot to remove. It could be quite embarrassing. I think it would be safer to skip any files not in git, except for directories added from the builddir (the documentation)
Comment 1 Carlos Garcia Campos 2014-07-10 10:00:34 PDT
Created attachment 234708 [details]
Patch
Comment 2 Martin Robinson 2014-07-10 10:30:44 PDT
Comment on attachment 234708 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234708&action=review

> Tools/gtk/make-dist.py:94
> +        self.files_in_cms = self.list_cms_files()

I'd prefer files_ins_vcs or files_in_version_control

> Tools/gtk/make-dist.py:104
> +        cmd = ['git', 'ls-tree', '-r', '--name-only', 'HEAD', self.source_root]
> +        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)

This could be one line. This doesn't really support SVN. Is there a script in the WebKit scripts that can do this for us?

> Tools/gtk/make-dist.py:129
> -                if not passes_all_rules(file):
> +                if not passes_all_rules(file) or self.should_skip_file(file):

Is this going to do the right thing for files from the build directory?
Comment 3 Carlos Garcia Campos 2014-07-10 23:13:30 PDT
(In reply to comment #2)
> (From update of attachment 234708 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234708&action=review
> 
> > Tools/gtk/make-dist.py:94
> > +        self.files_in_cms = self.list_cms_files()
> 
> I'd prefer files_ins_vcs or files_in_version_control

I actually meant scm :-P which is what webkitpy uses, but I don't really mind either vcs or version_control

> > Tools/gtk/make-dist.py:104
> > +        cmd = ['git', 'ls-tree', '-r', '--name-only', 'HEAD', self.source_root]
> > +        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> 
> This could be one line. This doesn't really support SVN. Is there a script in the WebKit scripts that can do this for us?

No, there isn't. It's very unlikely that someone will make a release without a git mirror, so I don't think it's worth it. I can leave a FIXME just in case someone want to add support for it (I won't)

> > Tools/gtk/make-dist.py:129
> > -                if not passes_all_rules(file):
> > +                if not passes_all_rules(file) or self.should_skip_file(file):
> 
> Is this going to do the right thing for files from the build directory?

Yes, it actually allows individual files when the source root is not in git (this is the case of files in buildir), assuming that in those cases they have been explicitly added by the manifest. We can try to be more restrictive here, and allow only files that are not in git when they are in the build dir.
Comment 4 Carlos Garcia Campos 2014-07-11 02:33:29 PDT
Created attachment 234750 [details]
Patch

Updated patch. Addressed review comments, and made the rule to skip files more restrictive to only allow files not in git when they are added from build dir.
Comment 5 Philippe Normand 2014-07-31 01:24:41 PDT
Comment on attachment 234750 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234750&action=review

> Tools/ChangeLog:9
> +        Skip all files in the source tree that are not under the control
> +        version, except for files add from the build dir like the documentation.

Nit: "not under version control" and "files added from..."
Comment 6 Carlos Garcia Campos 2014-07-31 01:41:58 PDT
Committed r171845: <http://trac.webkit.org/changeset/171845>