Source tarballs are being placed in git?

[Gerald B. Cox]

I received an email this AM from a fellow packager:




*Please, do not put source tarballs in git. This is not what is supposed to
happen. Package sources are managed with fedpkg, they are just added to the
lookaside cache and to the .gitignore file so you don't add them by mistake
to the repository.https://fedoraproject.org/wiki/Package_maintenance_guide
<https://fedoraproject.org/wiki/Package_maintenance_guide>How did you
manage to add them to the commit even if they are already in .gitignore?*

The package is megatools.  The procedure I used was from the maintenance
guide -
yet apparently, these files ended up in a place where they weren't suppose
to be.

The commands I used were:

   - *Stage any small patches or new source files for commit:*

*git add (somefile)
*

*Details *

*git does not consider all files in the working directory to be a part of
the git repository by default (handy, for keeping other files around that
are relevant, like the source tree). This tells git to start considering
these files as part of the repository locally. When you 'commit' and
'push'
later, this change is communicated to the server. *

   - * Upload new source files to the lookaside cache:*

*fedpkg new-sources
*

[Artur Iwicki]

That section of the guide is a bit poorly worded. You should *not* use "git add"
on source tarballs. These should be added only via means of "fedpkg new-sources
$FILES; git add ./sources". I believe what the guide means under "new source
files" is e.g. when upstream does not provide an icon or a .desktop or an
.appdata.xml file (or a systemd .service or whatever) and you add your own. This does not
include "new upstream release tarball".

I'll try to think of some way to make that more clear and submit a suggestion to
change the guide text.

[Todd Zullinger]

Artur Iwicki wrote:
>  That section of the guide is a bit poorly worded. You should *not*
> use "git add" on source tarballs. These should be added only via means of
> "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means
> under "new source files" is e.g. when upstream does not provide an icon or a
> .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own.
> This does not include "new upstream release tarball".
>  
>  I'll try to think of some way to make that more clear and submit a suggestion to
> change the guide text. 
Somewhat tangentially, it may be worth recommending that the
.gitignore be edited suitably for the project as one of the
first steps in maintainence.

By default 'fedpkg sources' will add the files being added
to the lookaside cache to the .gitignore, but it will never
remove older files, leading to an unnecessarily large
.gitignore file.

It's much more legible to use git's pattern matching
facilities to match the source files.

For example, the rpmlint's .gitignore contains the
following¹:

/*.rpm
/results_rpmlint/
/rpmlint-*/
/rpmlint-*.tar.gz

This keeps 'fedpkg source' from needing to add each new
tarball to the file.  More importantly, it makes it harder
to accidentally add a tarball to git.

The additional rules are convenient for ignoring other files
and directories which are often present in the work tree but
are never intended to tbe committed.

Refer to gitignore(5) for details on the pattern matching
rules.

¹ https://src.fedoraproject.org/rpms/rpmlint/blob/master/f/.gitignore

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If people are good only because they fear punishment, and hope for
reward, then we are a sorry lot indeed.
    -- Albert Einstein

[Daniel P. Berrangé]

On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
>  Artur Iwicki wrote:
>  > That section of the guide is a bit poorly worded. You should *not* use "git
> add" on source tarballs. These should be added only via means of "fedpkg
> new-sources $FILES; git add ./sources". I believe what the guide means under
> "new source files" is e.g. when upstream does not provide an icon or a .desktop
> or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This
> does not include "new upstream release tarball".
>  > 
>  > I'll try to think of some way to make that more clear and submit a suggestion to
> change the guide text.
>  
>  Somewhat tangentially, it may be worth recommending that the
>  .gitignore be edited suitably for the project as one of the
>  first steps in maintainence.
>  
>  By default 'fedpkg sources' will add the files being added
>  to the lookaside cache to the .gitignore, but it will never
>  remove older files, leading to an unnecessarily large
>  .gitignore file. 
It probably isn't unreasonable to file an RFE against fedpkg to request
more intelligent behaviour.  eg given a tarball filename in majority of
cases it is likely quite straightforward to identify the base name
eg match the filename against something like this

   (\w+)-(\d+(\.\d+)*).(zip|tar.\w+|tgz)

And replace the 2nd part of the regex match with '*' to form the rule
to add to the .gitignore file.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Tomasz Torcz]

On Tue, Jul 24, 2018 at 07:06:49PM +0100, Daniel P. Berrangé wrote:
>  On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
>  > Artur Iwicki wrote:
>  > > That section of the guide is a bit poorly worded. You should *not* use
> "git add" on source tarballs. These should be added only via means of
> "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means
> under "new source files" is e.g. when upstream does not provide an icon or a
> .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own.
> This does not include "new upstream release tarball".
>  > > 
>  > > I'll try to think of some way to make that more clear and submit a
> suggestion to change the guide text.
>  > 
>  > Somewhat tangentially, it may be worth recommending that the
>  > .gitignore be edited suitably for the project as one of the
>  > first steps in maintainence.
>  > 
>  > By default 'fedpkg sources' will add the files being added
>  > to the lookaside cache to the .gitignore, but it will never
>  > remove older files, leading to an unnecessarily large
>  > .gitignore file.
>  
>  It probably isn't unreasonable to file an RFE against fedpkg to request
>  more intelligent behaviour.  eg given a tarball filename in majority of
>  cases it is likely quite straightforward to identify the base name
>  eg match the filename against something like this
>  
>     (\w+)-(\d+(\.\d+)*).(zip|tar.\w+|tgz)
>  
>  And replace the 2nd part of the regex match with '*' to form the rule
>  to add to the .gitignore file. 
  Can't this be inferred from Sources: line? Just like rpmbuild does?

-- 
Tomasz Torcz            There exists no separation between gods and men:
xmpp: zdzichubg(a)chrome.pl   one blends softly casual into the other.

[Todd Zullinger]

Daniel P. Berrangé wrote:
>  On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
> > Artur Iwicki wrote:
> >> That section of the guide is a bit poorly worded. You should *not* use "git
> add" on source tarballs. These should be added only via means of "fedpkg
> new-sources $FILES; git add ./sources". I believe what the guide means under
> "new source files" is e.g. when upstream does not provide an icon or a .desktop
> or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This
> does not include "new upstream release tarball".
> >> 
> >> I'll try to think of some way to make that more clear and submit a suggestion
> to change the guide text.
> > 
> > Somewhat tangentially, it may be worth recommending that the
> > .gitignore be edited suitably for the project as one of the
> > first steps in maintainence.
> > 
> > By default 'fedpkg sources' will add the files being added
> > to the lookaside cache to the .gitignore, but it will never
> > remove older files, leading to an unnecessarily large
> > .gitignore file.
>  
>  It probably isn't unreasonable to file an RFE against fedpkg to request
>  more intelligent behaviour.  eg given a tarball filename in majority of
>  cases it is likely quite straightforward to identify the base name
>  eg match the filename against something like this
>  
>     (\w+)-(\d+(\.\d+)*).(zip|tar.\w+|tgz)
>  
>  And replace the 2nd part of the regex match with '*' to form the rule
>  to add to the .gitignore file. 
Yeah, perhaps someone will get bored and do that.  Years
ago, I submitted a patch to fedpkg/rpkg to have it resepct
the existing .gitignore, such that if you have a pattern
which matches the source being added, fedpkg won't add
another entry.

Another option would be to have fedpkg use the package N-V-R
to construct a gitignore entry and use that instead of the
filename if the filename matches the pattern.  I don't think
it would be too hard to generate a pattern using a regex
either, I just haven't been bothered enough to work on a
patch.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I personally think we developed language because of our deep need to
complain.
    -- Lily Tomlin

[Thomas Moschny]

2018-07-24 20:28 GMT+02:00 Todd Zullinger <tmz(a)pobox.com&gt;:
>  Years
>  ago, I submitted a patch to fedpkg/rpkg to have it resepct
>  the existing .gitignore, such that if you have a pattern
>  which matches the source being added, fedpkg won't add
>  another entry. 
My impression was that this is actually implemented in current fedpkg.
I have /foo-*.tgz patterns in many of the packages I maintain, and
fedpkg new-sources seems to respect that.

- Thomas

[Todd Zullinger]

Thomas Moschny wrote:
>  2018-07-24 20:28 GMT+02:00 Todd Zullinger <tmz(a)pobox.com&gt;:
> > Years
> > ago, I submitted a patch to fedpkg/rpkg to have it resepct
> > the existing .gitignore, such that if you have a pattern
> > which matches the source being added, fedpkg won't add
> > another entry.
>  
>  My impression was that this is actually implemented in current fedpkg.
>  I have /foo-*.tgz patterns in many of the packages I maintain, and
>  fedpkg new-sources seems to respect that. 
Yeah, it is.  The patch I mentioned was applied (in rpkg¹).
I could have stated that more clearly.

¹ f260d2e ("fedpkg: Try not to add redundant gitignore
  entries", 2010-09-08)

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The distinction between Freedom and Liberty is not accurately known;
naturalists have been unable to find a living specimen of either.
    -- Ambrose Bierce, "The Devil's Dictionary"

/results_* and /*.rpm in .gitignore (was: Source tarballs are being placed in git?)

[Tim Landscheidt]

Todd Zullinger <tmz(a)pobox.com&gt; wrote:

>  […] 
>  For example, the rpmlint's .gitignore contains the
>  following¹: 
>  /*.rpm
>  /results_rpmlint/
>  /rpmlint-*/
>  /rpmlint-*.tar.gz 
>  […] 
Apropos: Many .gitignores only reference the source files,
i. e. not /results_${name} or /${name}-*.rpm.  Therefore I
usually add the latter two to .git/info/exclude and I wonder
how others handle this.

Will fedpkg (and its backend) always create results_${name}
and ${name}-*.rpm in the top directory or is the destination
configurable?  If the former, it would make sense to add
them to .gitignore "for everybody", e. g. recommend that in
the Packaging Guidelines.  If not, I'd find it useful to
have "fedpkg clone" add them to the initial
.git/info/exclude.

Are there other approaches to have a clean "git status" dur-
ing "normal" packaging development, tests, etc.?

Tim

/results_* and /*.rpm in .gitignore (was: Source tarballs are being placed in git?)

[Todd Zullinger]

Tim Landscheidt wrote:
>  Todd Zullinger <tmz(a)pobox.com&gt; wrote:
>  
> > […]
>  
> > For example, the rpmlint's .gitignore contains the
> > following¹:
>  
> > /*.rpm
> > /results_rpmlint/
> > /rpmlint-*/
> > /rpmlint-*.tar.gz
>  
> > […]
>  
>  Apropos: Many .gitignores only reference the source files,
>  i. e. not /results_${name} or /${name}-*.rpm.  Therefore I
>  usually add the latter two to .git/info/exclude and I wonder
>  how others handle this. 
If it's a package I often work on, I generally add it to the
.gitignore directly.

>  Will fedpkg (and its backend) always create results_${name}
>  and ${name}-*.rpm in the top directory or is the destination
>  configurable?  If the former, it would make sense to add
>  them to .gitignore "for everybody", e. g. recommend that in
>  the Packaging Guidelines.  If not, I'd find it useful to
>  have "fedpkg clone" add them to the initial
>  .git/info/exclude. 
The results_* and *.src.rpm files are always created at the
top level and are not configurable in fedpkg/rpkg (so far as
I can tell).

I think it makes sense to have them in every package
.gitignore file.  Whether that's best done via the scripts
which are used to create a new repo after a package review,
via fedpkg/rpkg (automatically or by a command/option), or
just documented as a best practice in the guidelines I'm not
sure.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I have to decide between two equally frightening options.  If I wanted
to do that, I'd vote.
    -- Duckman

/results_* and /*.rpm in .gitignore

[Chenxiong Qi]

On 07/25/2018 11:59 PM, Todd Zullinger wrote:
>  Tim Landscheidt wrote:
> > Todd Zullinger <tmz(a)pobox.com&gt; wrote:
> >
> >> […]
> >
> >> For example, the rpmlint's .gitignore contains the
> >> following¹:
> >
> >> /*.rpm
> >> /results_rpmlint/
> >> /rpmlint-*/
> >> /rpmlint-*.tar.gz
> >
> >> […]
> >
> > Apropos: Many .gitignores only reference the source files,
> > i. e. not /results_${name} or /${name}-*.rpm.  Therefore I
> > usually add the latter two to .git/info/exclude and I wonder
> > how others handle this.
>  
>  If it's a package I often work on, I generally add it to the
>  .gitignore directly.
>  
> > Will fedpkg (and its backend) always create results_${name}
> > and ${name}-*.rpm in the top directory or is the destination
> > configurable?  If the former, it would make sense to add
> > them to .gitignore "for everybody", e. g. recommend that in
> > the Packaging Guidelines.  If not, I'd find it useful to
> > have "fedpkg clone" add them to the initial
> > .git/info/exclude.
>  
>  The results_* and *.src.rpm files are always created at the
>  top level and are not configurable in fedpkg/rpkg (so far as
>  I can tell).
>  
>  I think it makes sense to have them in every package
>  .gitignore file.  Whether that's best done via the scripts
>  which are used to create a new repo after a package review,
>  via fedpkg/rpkg (automatically or by a command/option), or
>  just documented as a best practice in the guidelines I'm not
>  sure.
>   
fedpkg also generates other directories and files from commands, e.g. 
prep or local.

fedpkg could write patterns to .git/info/exclude just after clone, which 
ignore known directories and files generated by its commands.

For other possible ignore patterns, they could be defined in user config 
file ~/.config/rpkg/fedpkg.conf that would be used in all packages that 
packager maintains.

To summarize, after package is cloned,

1. fedpkg writes known patterns to .git/info/exclude immediately
2. read predefined patterns from user config file, and if there is, 
write to .git/info/exclude as well.

Is this viable?

>  
>  
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail].o...
>   
-- 
Regards,
Chenxiong Qi

[Zbigniew Jędrzejewski-Szmek]

On Tue, Jul 24, 2018 at 05:07:50PM -0000, Artur Iwicki wrote:
>  That section of the guide is a bit poorly worded. You should *not*
>  use "git add" on source tarballs. These should be added only via
>  means of "fedpkg new-sources $FILES; git add ./sources". 
Actually "fedpkg new-sources" already does "git add" for you.
I consider this an anti-feature (it "break" 'git diff', making
'git diff HEAD' or such necessary), but anyway, calling
'git add sources' is not necessary.

Zbyszek

[Gerald B. Cox]

On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve(a)fedoraproject.org&gt;
wrote:

>  That section of the guide is a bit poorly worded. You should *not*
> use
>  "git add" on source tarballs. These should be added only via means of
>  "fedpkg new-sources $FILES; git add ./sources". I believe what the guide
>  means under "new source files" is e.g. when upstream does not provide an
>  icon or a .desktop or an .appdata.xml file (or a systemd .service or
>  whatever) and you add your own. This does not include "new upstream release
>  tarball".
>
>  I'll try to think of some way to make that more clear and submit a
>  suggestion to change the guide text.
>  
Thanks for the clarification and taking the initiative to get it fixed.  As
far as "poorly worded", that's a candidate
for understatement of the day... LOL

It uses the exact same wording - "new source files" in both statements with
absolutely no differentiation:

- Stage any small patches or new source files for commit:
- Upload new source files to the lookaside cache:

[Vít Ondruch]

Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
>  On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki
> <suve(a)fedoraproject.org
>  <mailto:[e-mail]>> wrote:
>
>      That section of the guide is a bit poorly worded. You should *not*
>      use "git add" on source tarballs. These should be added only via
>      means of "fedpkg new-sources $FILES; git add ./sources".
>  
"new-sources" should not be mentioned in basic documentation. These
should be added via means of "fedpkg import package.srpm".

The workflow for new package is then:

~~~
$ fedpkg import mypackage.srpm
$ git commit -m "initial import"
~~~

and for updated package:

~~~
$ git add -u
$ fedpkg clog
$ git commit -F clog
$ fedpkg import updated.srpm
$ git commit --am
~~~

This way, the question if the file is stored in lookaside cache or git
is done by fedpkg. Also please note that the "import" command removes
patches which are not used any longer (not sure if it adds new patches ...).


V.

>      I believe what the guide means under "new source files"
> is e.g.
>      when upstream does not provide an icon or a .desktop or an
>      .appdata.xml file (or a systemd .service or whatever) and you add
>      your own. This does not include "new upstream release tarball".
>
>      I'll try to think of some way to make that more clear and submit a
>      suggestion to change the guide text.
>
>
>  Thanks for the clarification and taking the initiative to get it
>  fixed.  As far as "poorly worded", that's a candidate
>  for understatement of the day... LOL
>
>  It uses the exact same wording - "new source files" in both statements
>  with absolutely no differentiation:
>
>  - Stage any small patches or new source files for commit:
>  - Upload new source files to the lookaside cache:
>
>
>
>
>
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail].o...

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 25, 2018 at 01:48:58PM +0200, Vít Ondruch wrote:
>  
>  
>  Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
>  > On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve(a)fedoraproject.org
>  > <mailto:[e-mail]>> wrote:
>  >
>  >     That section of the guide is a bit poorly worded. You should *not*
>  >     use "git add" on source tarballs. These should be added only via
>  >     means of "fedpkg new-sources $FILES; git add ./sources".
>  >
>  
>  "new-sources" should not be mentioned in basic documentation. These
>  should be added via means of "fedpkg import package.srpm".
>  
>  The workflow for new package is then:
>  
>  ~~~
>  $ fedpkg import mypackage.srpm
>  $ git commit -m "initial import"
>  ~~~
>  
>  and for updated package:
>  
>  ~~~
>  $ git add -u
>  $ fedpkg clog
>  $ git commit -F clog
>  $ fedpkg import updated.srpm
>  $ git commit --am
>  ~~~ 
Why make two separate commits here?

Zbyszek

[Vít Ondruch]

Dne 25.7.2018 v 13:53 Zbigniew Jędrzejewski-Szmek napsal(a):
>  On Wed, Jul 25, 2018 at 01:48:58PM +0200, Vít Ondruch wrote:
> >
> > Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
> >> On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve(a)fedoraproject.org
> >> <mailto:[e-mail]>> wrote:
> >>
> >>     That section of the guide is a bit poorly worded. You should *not*
> >>     use "git add" on source tarballs. These should be added only via
> >>     means of "fedpkg new-sources $FILES; git add ./sources".
> >>
> > "new-sources" should not be mentioned in basic documentation. These
> > should be added via means of "fedpkg import package.srpm".
> >
> > The workflow for updated package:
> >
> > ~~~
> > $ git add -u
> > $ fedpkg clog
> > $ git commit -F clog
> > $ fedpkg import updated.srpm
> > $ git commit --am
> > ~~~
>  Why make two separate commits here? 
If I am not mistaken, you cannot import if your git repository is not in
clean state, so you would have to reset the repository. Also, the files
in the repository are in state I want to commit, but the SRPM might be a
bit obsolete, typically due to last second changes of changelog. So the
"commit" and later "commit --amend" is safe workflow IMO.


V.

[Gerald B. Cox]

On Wed, Jul 25, 2018 at 4:48 AM, Vít Ondruch <vondruch(a)redhat.com&gt; wrote:

>  "new-sources" should not be mentioned in basic documentation. These should
>  be added via means of "fedpkg import package.srpm".
>
>  The workflow for new package is then:
>
>  ~~~
>  $ fedpkg import mypackage.srpm
>  $ git commit -m "initial import"
>  ~~~
>
>  and for updated package:
>
>  ~~~
>  $ git add -u
>  $ fedpkg clog
>  $ git commit -F clog
>  $ fedpkg import updated.srpm
>  $ git commit --am
>  ~~~
>
>  This way, the question if the file is stored in lookaside cache or git is
>  done by fedpkg. Also please note that the "import" command removes patches
>  which are not used any longer (not sure if it adds new patches ...).
>
>
>  
Thanks.  If fedpkg has this capability IMO it should be referenced in the
package maintenance guide.  I understand that documentation is boring, but
if someone makes a change or adds capability to a package, they need to
take a few moments to update references to those capabilities in the
documentation - especially if that documentation is something as important
as the "package maintenance guide".  I reference the guide every single
time I make changes to packages - just to ensure I'm following the current
rules.  At least for me, it's vital that these instructions are current and
correct.