Created attachment 151441 [details] Proposed patch (since 353967 revision) Patch to update devel/tig port from 2.0.2 to 2.0.3 version. Look following links for changes: http://jonas.nitro.dk/tig/NEWS.html https://github.com/jonas/tig/compare/tig-2.0.2...tig-2.0.3 - Add LICENSE_FILE - Strip bin/tig by adding "strip" before "install" for INSTALL_TARGET - Remove default target(s) from MANPAGES_ALL_TARGET and MANPAGES_INSTALL_TARGET, which used to add additional target(s)
Maintainer CC'd
Created attachment 151442 [details] The poudriere testport log (FreeBSD 10 amd64)
maintainer timeout. patch is poudriere-tested and looks good, so I'm moving to patch-ready without maintainer approval. Note: the poudriere log is warmly welcomed, but please don't gzip logs when they are less than 1Mb in size. We want to view them in a browser.
-MANPAGES_ALL_TARGET= all doc-man -MANPAGES_INSTALL_TARGET=install install-doc-man +MANPAGES_ALL_TARGET= doc-man +MANPAGES_INSTALL_TARGET= install-doc-man This is incorrect. Due to implementation in ports framework, XXX_YYY_TARGET from options override, not append default targets. So with MANPAGES enabled, ALL_TARGET will be doc-man and INSTALL_TARGET will be install-doc-man, with `all' and `install' list. +INSTALL_TARGET= strip install Won't this be better? +ALL_TARGET= all strip
(In reply to John Marino from comment #3) > Note: the poudriere log is warmly welcomed, but please don't gzip logs when they are less than 1Mb in size. We want to view them in a browser. Ok, but this might increase demands on server capacity (in case of huge files), if there is no automatic server-side compression for text files, about which I don't know.
Created attachment 152024 [details] Proposed patch (since 353967 revision) (In reply to Dmitry Marakasov from comment #4) > This is incorrect. Due to implementation in ports framework, XXX_YYY_TARGET from options override, not append default targets. So with MANPAGES enabled, ALL_TARGET will be doc-man and INSTALL_TARGET will be install-doc-man, with `all' and `install' list. Not really, look Mk/bsd.options.mk file: # For each of: # ALL_TARGET CATEGORIES CFLAGS CONFIGURE_ENV CONFLICTS CONFLICTS_BUILD # CONFLICTS_INSTALL CPPFLAGS CXXFLAGS DISTFILES EXTRA_PATCHES # INSTALL_TARGET LDFLAGS LIBS MAKE_ARGS MAKE_ENV PATCHFILES PATCH_SITES # PLIST_DIRS PLIST_DIRSTRY PLIST_FILES PLIST_SUB INFO USES, defining ${opt}_${variable} will # add its content to the actual variable when the option is enabled. Defining # ${opt}_${variable}_OFF will add its content to the actual variable when the # option is disabled. #... _OPTIONS_FLAGS= ALL_TARGET CATEGORIES CFLAGS CONFIGURE_ENV CONFLICTS \ CONFLICTS_BUILD CONFLICTS_INSTALL CPPFLAGS CXXFLAGS DISTFILES \ EXTRA_PATCHES INSTALL_TARGET LDFLAGS LIBS MAKE_ARGS MAKE_ENV \ PATCHFILES PATCH_SITES PLIST_DIRS PLIST_DIRSTRY PLIST_FILES \ PLIST_SUB USES INFO #... . for flags in ${_OPTIONS_FLAGS} . if defined(${opt}_${flags}) ${flags}+= ${${opt}_${flags}} . endif #... . for flags in ${_OPTIONS_FLAGS} . if defined(${opt}_${flags}_OFF) ${flags}+= ${${opt}_${flags}_OFF} . endif While you are right about default values, if they didn't touched, according to Mk/bsd.port.mk file: # ALL_TARGET - Default target for sub-make in build stage. # Default: all #... # INSTALL_TARGET # - Default target for sub-make in install stage. # Default: install because of ?=: ALL_TARGET?= all INSTALL_TARGET?= install Where I override the INSTALL_TARGET with INSTALL_TARGET= strip install which strips the binary before install, what the poudriere testport log clearly shows. It's possible to add the ALL_TARGET also: ALL_TARGET= all which gives the following values in case of MANPAGES option enabled: # make -V ALL_TARGET -V INSTALL_TARGET all doc-man strip install install-doc-man and without: # make -V ALL_TARGET -V INSTALL_TARGET all strip install The new proposed patch attached. Thanks for the hint.
Created attachment 152025 [details] The poudriere testport log (FreeBSD 10 amd64)
(In reply to lightside from comment #5) > about which I don't know. The ZFS file system might do this, if this is a case.
Created attachment 152028 [details] Proposed patch (since 353967 revision) (In reply to Dmitry Marakasov from comment #4) > +INSTALL_TARGET= strip install > Won't this be better? > > +ALL_TARGET= all strip Yes, this is better, in case of restage.
Created attachment 152029 [details] The poudriere testport log (FreeBSD 10 amd64)
(In reply to John Marino from comment #3) I need to note, that compressed log files are better option, because of integrity checking. Also, this saves bandwidth of the downloader. Most of the archives extracted with "tar -xf archive" command. There could be many such archived log files with different name, but with the same name in contents, intended to be extracted in different directories. Viewing the patch in the browser is different story, where there is diff option. For example, the Firefox/Seamonkey browser downloads *.log files anyway, by default. So, I think, this is not a mandatory requirement.
Created attachment 152037 [details] The poudriere testport log (FreeBSD 10 amd64) Renaming *.log to *.txt might do the "trick", if the integrity of log is not so important.
(In reply to comment #12) > Renaming *.log to *.txt might do the "trick" or changing mime type of the file to "plain text (text/plain)". Sorry for off-topic.
> Not really, look Mk/bsd.options.mk file: Yes, you are right and your version looks more correct. Committed, thanks!
A commit references this bug: Author: amdmi3 Date: Fri Jan 23 11:44:01 UTC 2015 New revision: 377726 URL: https://svnweb.freebsd.org/changeset/ports/377726 Log: - Update to 2.0.3 - Add LICENSE_FILE - Strip binary PR: 196582 Submitted by: lightside@gmx.com Approved by: maintainer timeout Changes: head/devel/tig/Makefile head/devel/tig/distinfo
(In reply to lightside from comment #5) > (In reply to John Marino from comment #3) > > Note: the poudriere log is warmly welcomed, but please don't gzip logs when > they are less than 1Mb in size. We want to view them in a browser. > > Ok, but this might increase demands on server capacity (in case of huge files), > if there is no automatic server-side compression for text files, about which I > don't know. You don't have to worry about that. It will not allow an upload bigger than 1Mb, which is considered an acceptable size > I need to note, that compressed log files are better option, because of > integrity checking. I do not care about log integrity. > Also, this saves bandwidth of the downloader. The downloader is the one requesting a plain-text attachment. > Most of the archives extracted with "tar -xf archive" command. Irrelevant > There could be > many such archived log files with different name, but with the same name in > contents, intended to be extracted in different directories. The consumer of the logs wants to see one file per log, and wants to see it in plain text, if it's less than 1Mb. If that means multiple attachments, so be it. If that means a link to where logs can be downloaded in plain text, that's acceptable (and fairly common). > Viewing the patch in the browser is different story, where there is diff option. We are not talking about patches (which should be designated as patches during the upload. I'm talking about logs. > For example, the Firefox/Seamonkey browser downloads *.log files anyway, by > default. So, I think, this is not a mandatory requirement. No, that's not the browser doing that, it's the web server. I can view files with .log extension just fine if the webserver has sane settings. Nothing is mandatory. For example, I don't have to review an PR that makes review more difficult than it has to be.