Created attachment 146362 [details] ja-eijiro-fpw.diff Stage-fy japanese/eijiro-fpw. Fix: found some warnings but no problem with a patch attached: % pwd /usr/ports/japanese/eijiro-fpw % make check-plist ====> Checking for pkg-plist issues (check-plist) ===> Parsing plist ===> Checking for items in STAGEDIR missing from pkg-plist ===> Checking for directories owned by MTREEs ===> Checking for directories handled by dependencies ===> Checking for items in pkg-plist which are not in STAGEDIR ===> No pkg-plist issues found (check-plist) % make stage-qa ====> Running Q/A tests (stage-qa) % portlint WARN: Makefile: [34]: use of != in assignments is almost never a good thing to do. Try to avoid using them. See http://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html for some helpful hints on what to do instead. WARN: Makefile: possible use of absolute pathname "/cdrom". WARN: Makefile: using hyphen in PORTNAME. consider using PKGNAMEPREFIX and/or PKGNAMESUFFIX. WARN: Makefile: Consider defining LICENSE. WARN: /usr/ports/japanese/eijiro-fpw/files/patch-aa: [4]: patch contains ^M characters. Consider defining USES=dos2unix to remove DOS line endings from source files. 0 fatal errors and 5 warnings found.
I don't like this DICT_ARCHIVE!= line that portlint is complaining out. This is a staged directory so the existence of files are controlled. We shouldn't need to check for that. Can you take this back and see if the "!=" is either 1) unnecessary 2) necessary (please explain why) but then try to figure out a different way that doestn't involve a shell command.
I think this is necessary. we must check whether a special file exist before entering any target, for in some case, port must depend on some other port to extract the archive. I think "EXTRACT_DEPENDS=" must be in prior to any target, is it right ? users who want to use this port must have a data normally stored in a cd-rom, and there are many types of cd-rom versions with different directory tree... In addition, shell command in this ports merely runs "echo", so it is safe even if it was run many times. If we must avoid this shell command outside of any targets, I think port should EXTRACT_DEPENDS on lha in any cases. do you think which is better ?
(In reply to turutani from comment #2) > I think this is necessary. > we must check whether a special file exist before entering any target, But this is STAGEDIR. We control exactly what exists. It's never an unknown. So we should know if DICT_ARCHIVE exists or not. It will always be either true or false in the STAGEDIR depending on the build dependencies, right? > If we must avoid this shell command outside of any targets, > I think port should EXTRACT_DEPENDS on lha in any cases. > do you think which is better ? I would (and if you can't maybe I will) use "poudriere testport -i" to see exactly what is in the stagedir. If the DICT_ARCHIVE is present, you always add it. If it's not, we remove the line.
i think this is not in the STAGEDIR... once original dict file (${DICT_PATH}/${SRCFILE}) is processed, resulting file is settled in the ${WRKSRC} in any cases. Installation to the stage dir begins after then.
wait, if we can't be sure about the input, how can we be sure about the output? Are you saying the known input files are sometimes compressed and sometimes not? This is a strange port.
okay, look at the post-extract target: post-extract: @if [ -f ${CDROM_PATH}/eijiro-original/*.exe ]; then \ lha xiw=${WRKDIR} ${CDROM_PATH}/eijiro-original/*.exe '*/${SRCFILE}'; \ elif [ ! -f ${DICT_PATH}/${SRCFILE} ]; then \ ${ECHO} "###################################################"; \ ${ECHO} "I cannot find a file ${DICT_PATH}/${SRCFILE}"; \ ${ECHO} "Specify the Eijiro Dictionary file with the full path"; \ ${ECHO} "and excecute the following command again:"; \ ${ECHO} " make DICT_PATH=${DICT_PATH} SRCFILE=${SRCFILE}"; \ ${ECHO} ""; \ ${ECHO} "Notice: DICT_PATH must be a full path to the file."; \ ${ECHO} "###################################################"; \ ${FALSE}; \ fi ${CP} ${LOCALBASE}/share/freepwing/fpwutils.mk ${WRKSRC} So it fails unless it can uncompress data. Therefore, lha is unconditionally required. It makes no sense to require it only if the data can be found. Do you see what I mean? What's the point? Just to save building a dependency if the port is going to fail at post-extract anyway?
I do not know about this... As far as I know, some versions of CD-ROM are sold. At least one of them (which I have) contains compressed data with self-extracting lha archive file, having .exe suffix. On the other hand, using decompressed data is very useful for users without optical drive and for ones who can obtaine data from other method; hence SRCFILE and DICT_PATH can be set by users. Therefore, it is useful for us to treat both types of data, even though the Makefile is something confusing... as I guess.
(In reply to turutani from comment #7) > I do not know about this... > As far as I know, some versions of CD-ROM are sold. > At least one of them (which I have) contains compressed data with > self-extracting lha archive file, having .exe suffix. Okay, I looked closer and it's trying to handle both cases (either compressed, or uncompressed eijiro52.txt. Let's just make this Makefile simpler: A) Make lha an unconditional EXTRACT_DEPENDS, and move it where the first EXTRACT_DEPENDS is defined. B) Get rid of ${CDROM_PATH} completely as well as the DIST_ARCHIVE check the SRCFILE is always going to end up in WRKDIR directory, so change MAKE_ARGS+= SRCFILE=${WRKDIR}/${SRCFILE} Then we can leave the post-extract target mostly how it is now. [1] Doing it this way makes it simpler, removes the shell command, and the only downside is an unused dependency on lha in the case of an uncompresses source file, but that's not a big penalty, right? what do you think? [1] I think "-f ${CDROM_PATH}.." changes to "-f ${DICT_pATH}.." and on the next line "-f ${DICT_PATH}/${SRCFILE}" changes to "-f ${WRKDIR}/${SRCFILE}"
Created attachment 146477 [details] new patch I would rather like this way. if someone who need extract data from CD-ROM, please do it yourself, as the message shown. I think this is much simple.
(In reply to turutani from comment #9) > Created attachment 146477 [details] > new patch > > I would rather like this way. > if someone who need extract data from CD-ROM, please do it yourself, > as the message shown. > I think this is much simple. Yeah, but then it does less than before. Plus your new patch didn't put a default to DICT_PATH. I'll take this PR and fix it as I described (which is simpler than before but more complex than this)
if I have a dictionary file already decompressed, should i put them into WRKSRC everytime I want to build it ?
you don't have too, you just override the DICT_PATH. But here you removed the default value. And I understood that compressed files were common. Are they not common?
I found my CD-ROM ! A compressed archive file contains four dictionary files, each of them for eijiro-fpw, waeijiro-fpw, otojiro-fpw, ryaku-fpw. The uncompressed file for eijiro-fpw and waeijiro-fpw have more than 50MB. The compressed archive file is named with multi-byte charactors...
Okay, I think your said your cdrom had one compressed archive that expands into 4 dictionary files, each used in a different port. Which means we keep the decompression support, right?
yes, please do as you think; I have no worries under any cases.
A commit references this bug: Author: marino Date: Fri Aug 29 21:27:29 UTC 2014 New revision: 366566 URL: http://svnweb.freebsd.org/changeset/ports/366566 Log: Stage japanese/eijiro-fpw I modernized and simplified this port beyond the PR, including embedding the pkg-plist and converting patch-aa with DOS line endings and using dos2unix. I also removed the shell command at the small expense of making lha a permanent dependency. The downside is that this is a manual package with a restricted cdrom, so I can't test any of it. I'll commit this with the hope that it's correct and request the submitter, who does own the restricted data, verify that it's working as hoped. PR: 193061 Submitted by: turutani (kyoto) Changes: head/japanese/eijiro-fpw/Makefile head/japanese/eijiro-fpw/files/patch-Makefile head/japanese/eijiro-fpw/files/patch-aa head/japanese/eijiro-fpw/pkg-plist
I have no way to test this. Can you please verify that both the compressed and uncompressed methods work? (copy SRCFILE to a different location to test the latter) Thanks. Hopefully it works, but if it doesn't, please provide a patch to fix my error.
Created attachment 146514 [details] Makefile.diff need some fix: fix typo. check the file specified by option args first. stops with message when the SRCFILE is not obtained from archive. please change Makefile with the attached patch.
Created attachment 146518 [details] Makefile.diff Sorry, previous one is wrong.
The typo was fixed maybe 60 seconds after the first commit, so you patch isn't based on the current port. However, I don't understand the changes. Lines 40-45 accomplish the same thing as the original version. There's no change in functionality that I can see. Line 53 *does* change functionality, but it appears wrong. The extraction logic is: A) are *.exe files detected? yes=> extract $SRCFILE to WRKSRC, no=> go to B) B) is the $SRCFILE already in WRKSRC? yes=> continue, no=> print error msg Lines 40-45 adjust MAKE_ARGS for cases A) and B) So the typo is already fixed, and it seems to me the logic is already good. Did you test the current port with your cdrom and found it doesn't work?
Okay, I see now you changed lines 40-45 to define SRC_FILE But the change in 53 (which uses SRC_FILE) isn't necessary. The extraction step in A) is assumed to work and B) covers the provided SRCFILE case. AFAICT, the current port (with the typo already fixed) should work for you right now.
If CD-ROM is found, which dictionary file is used ? in your patch, 40th line is always false in case A) and B). I tried with CD-ROM, and found it fails indeed. In addition, in case A), what happens if compressed archive file does not contain SRCFILE inside; the default value of SRCFILE is for the CD-ROM of the 1st edition. Hence, I think we should confirm the existance of the SECFILE.
(In reply to turutani from comment #22) > If CD-ROM is found, which dictionary file is used ? > in your patch, 40th line is always false in case A) and B). > I tried with CD-ROM, and found it fails indeed. It shouldn't fail. line 40 is not needed until the "do-install" target. the post-extract target will uncompress the cdrom and place the source file at WRKDIR. Ah, maybe "make clean; make extract ; make install" works but "make clean; make install" does not. That would be interesting to know. But what you had is no different? > In addition, in case A), what happens if compressed archive file > does not contain SRCFILE inside; the default value of SRCFILE is > for the CD-ROM of the 1st edition. Hence, I think we should confirm > the existance of the SECFILE. Hmmm. Well, nothing much would happen, it would just fail. I got your point though. Let me look at that part again.
Created attachment 146528 [details] slightly modified Okay, I see now what you are doing and agree with it. I slightly modified the patch, mainly I added :Q and comments. (There might be spaces in path we need to consider) Can you test that I didn't break anything?
Here is the output with previous Makefile: ------- start logging ------- % make clean ===> Cleaning for ja-eijiro-fpw-1.0b_5 % make extract ===> Found saved configuration for ja-eijiro-fpw-1.0b_4 ===> ja-eijiro-fpw-1.0b_5 depends on file: /usr/local/sbin/pkg - found ===> Fetching all distfiles required by ja-eijiro-fpw-1.0b_5 for building ===> Extracting for ja-eijiro-fpw-1.0b_5 ===> Found saved configuration for ja-eijiro-fpw-1.0b_4 ===> ja-eijiro-fpw-1.0b_5 depends on file: /usr/local/sbin/pkg - found ===> Fetching all distfiles required by ja-eijiro-fpw-1.0b_5 for building => SHA256 Checksum OK for eijiro-fpw1.0b-src.tar.gz. ===> ja-eijiro-fpw-1.0b_5 depends on file: /usr/local/share/freepwing/fpwutils.mk - found ===> ja-eijiro-fpw-1.0b_5 depends on executable: lha - found /usr/ports/japanese/eijiro-fpw/work/eijiro52.txt - Melted : o /bin/cp /usr/local/share/freepwing/fpwutils.mk /usr/ports/japanese/eijiro-fpw/work/eijiro-fpw1.0b % make install ===> Patching for ja-eijiro-fpw-1.0b_5 ===> Converting DOS text files to UNIX text files ===> Applying FreeBSD patches for ja-eijiro-fpw-1.0b_5 ===> ja-eijiro-fpw-1.0b_5 depends on file: /usr/local/libexec/freepwing/catdump - found ===> ja-eijiro-fpw-1.0b_5 depends on package: ja-p5-Jcode>=0 - found ===> ja-eijiro-fpw-1.0b_5 depends on executable: gmake - found ===> ja-eijiro-fpw-1.0b_5 depends on file: /usr/local/bin/perl5.16.3 - found ===> Configuring for ja-eijiro-fpw-1.0b_5 ===> Building for ja-eijiro-fpw-1.0b_5 Makefile:60: warning: overriding recipe for target `package' fpwutils.mk:478: warning: ignoring old recipe for target `package' test -d work || /usr/local/libexec/freepwing/mkdirhier work /usr/local/libexec/freepwing/perl.sh /usr/local/libexec/freepwing/fpwhalfchar \ -workdir work /usr/local/libexec/freepwing/perl.sh /usr/local/libexec/freepwing/fpwfullchar \ -workdir work /usr/local/libexec/freepwing/perl.sh eijiro-fpw.pl \ -workdir work -c "" "/cdrom/eijiro52.txt" Detected display character set: euc Can't open /cdrom/eijiro52.txt: No such file or directory at eijiro-fpw.pl line 60. Total 0 entries were written. Writing copyright information... /usr/local/libexec/freepwing/perl.sh /usr/local/libexec/freepwing/fpwsort -workdir work /usr/local/libexec/freepwing/perl.sh /usr/local/libexec/freepwing/fpwindex -workdir work Modification of non-creatable array value attempted, subscript -1 at /usr/local/lib/perl5/site_perl/5.16/mach/FreePWING/Index.pm line 226. gmake: *** [work/index.dep] Error 255 *** Error code 1 Stop in /usr/ports/japanese/eijiro-fpw. *** Error code 1 Stop in /usr/ports/japanese/eijiro-fpw. *** Error code 1 Stop in /usr/ports/japanese/eijiro-fpw. ------- end logging ------- Your new fix works fine ! Thank you for your enduring help. I will post the new fix about japanese/waeijiro-fpw ASAP.
A commit references this bug: Author: marino Date: Sat Aug 30 13:41:14 UTC 2014 New revision: 366618 URL: http://svnweb.freebsd.org/changeset/ports/366618 Log: japanese/eijiro-fpw: Small fix to cover compressed case The previous change was tested and needs a small patch to work in all cases. Patch functioning confirmed by PR submitter PR: 193061 Submitted by: turutani (kyoto) Changes: head/japanese/eijiro-fpw/Makefile
Thanks for testing, updated patch and final confirmation!