From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52615) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d9x7l-0003Je-Vc for guix-patches@gnu.org; Sun, 14 May 2017 13:16:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d9x7i-0002Je-QM for guix-patches@gnu.org; Sun, 14 May 2017 13:16:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:42365) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1d9x7i-0002JO-Km for guix-patches@gnu.org; Sun, 14 May 2017 13:16:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1d9x7i-00055a-99 for guix-patches@gnu.org; Sun, 14 May 2017 13:16:02 -0400 Subject: bug#26802: Single source file emacs packages get a ".el.el" extension Resent-Message-ID: From: Alex Kost References: <05a79dd0.AEAAJ6TpV0QAAAAAAAAAAAOtUOAAAAACwQwAAAAAAAW9WABZDccC@mailjet.com> <87wp9pbz2b.fsf@gmail.com> <19fd8da9.AEMAKMfGZKsAAAAAAAAAAAO9aM4AAAACwQwAAAAAAAW9WABZFLlP@mailjet.com> <87vap5xhks.fsf@gmail.com> <9b375d38.AEAAKIA9bmkAAAAAAAAAAAO9aM4AAAACwQwAAAAAAAW9WABZFzYx@mailjet.com> Date: Sun, 14 May 2017 20:15:36 +0300 In-Reply-To: <9b375d38.AEAAKIA9bmkAAAAAAAAAAAO9aM4AAAACwQwAAAAAAAW9WABZFzYx@mailjet.com> (Arun Isaac's message of "Sat, 13 May 2017 22:06:05 +0530") Message-ID: <8737c7fjgn.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Arun Isaac Cc: 26802@debbugs.gnu.org Arun Isaac (2017-05-13 22:06 +0530) wrote: > I've pushed the other uncontroversial patches. > >> It would be a linter only for *.el files, is that what you mean? AFAIK >> we do not have such specific linters, but yeah, why not, I think it's a >> good idea. > > No, it will be a linter for all packages, not just emacs packages. Hm, but the ".el" source files are special: they must have "name-version.el" name, while the other sources may be named pretty arbitrary. > The following is the current linter for checking source file names. I > have a couple of issues with it, in addition to the new linter feature I > am suggesting. Let me explain. > >> (define (check-source-file-name package) >> "Emit a warning if PACKAGE's origin has no meaningful file name." >> (define (origin-file-name-valid? origin) >> ;; Return #t if the source file name contains only a version or is #= f; ^^ (it's a typo I think, should be =E2=80=98#f=E2=80=99= there) >> ;; indicates that the origin needs a 'file-name' field. > > Isn't this logic somehow backward? Should'nt a predicate called > `file-name-valid?' return #t if the file name is valid, and #f > otherwise? This seems to be doing the opposite of that, returning #f if > the file name is valid, and #t otherwise. If I understand correctly, this predicate does the right thing; it's just a typo in the commentary. >> (let ((file-name (origin-actual-file-name origin)) >> (version (package-version package))) >> (and file-name >> (not (or (string-prefix? version file-name) >> ;; Common in many projects is for the filename to st= art >> ;; with a "v" followed by the version, >> ;; e.g. "v3.2.0.tar.gz". >> (string-prefix? (string-append "v" version) file-nam= e)))))) > > I think this check can be done by matching against a single regexp like > so: > > (string-match (string-append "^v?" version) file-name) I agree, a single regexp looks better! > In addition to this logic, we add an extra condition that makes sure the > version is some substring of the source file name, like so: > > (string-match version file-name) > > With this new check, single source file packages like > emacs-transpose-frame, etc. which did not have file-name fields would > have raised a lint warning. I'm not sure, I think: - it's too much for all the sources, as the upstream source may not contain a version in the file name at all. Do we really want to raise a warning in this case? - and it's not enough for ".el" sources, I mean "something-version.el" is not enough, as the file name must exactly be "name-version.el" (as it is with ELPA single-filed sources), so the emacs-build-system will output "name.el" file which will correspond to 'name' feature provided by this file. [...] >> P.S. Could you please keep my email in Cc? It would be easier to >> follow the discussion for me, thanks. > > Sorry, I thought debbugs does some magic to send out mails to everybody > involved in a specific bug. Didn't realize it was only using Cc. Will > keep you in Cc for this and future mails. Thanks! --=20 Alex