[Nix-dev] Re[2]: [Nix-commits] SVN commit: trace - 11216 - raskin - in nixpkgs/trunk/pkgs/misc/tex:. texlive

[Michael Raskin]

> - It is not indented in the proper way (and it uses tabs...).  For instance, the 
> big "let localDefs" is indented, which it shouldn't be, and readability would be 
> improved by separating the let-block from the surrounding code with empty lines.

OK, OK. Fixed it locally, will soon commit. I just dislike spaces-instead-of-tabs.
I even didn't make a tab/space mix in the expression.

> - There is one gigantic doInstall phase.  Why not use the generic builder in the 
> right way?  (Or is that because of texmfSrc?)
Because it was a work-in-progress. Or work in staggering and stumbling phase.

> - I don't understand all the strange "null" values, e.g. in "with builderDefs 
> {src="";} null;" (and what does src="" do anyway?) and in "let localDefs = 
> builderDefs (rec { ... }) args null; /* null is a terminator for sumArgs */".

OK, {src="";} refactored out. Actually, builderDefs defines some things that are useful even while building its arguments, but I was not sure if it was worth the effort to separate them (and have to think what will be useful and what not). So I just created a skeleton builderDefs attrSet to use these definitions. null is terminator for sumArgs, that is true - sumArgs takes a function, then takes arbitrary number of attrSets to combine into arguments for the function, and then does actual application when it gets null.

> - What does FullDepEntry do?

Composes a phase with non-trivial action and non-empty dependencies. You could always spell out text= and deps=, but I find it less convenient.

> - Why is configureFlags set to [], but then configure is called with a long of 
> arguments?

Experimental code and trivial to fix once it is relatively stable. Fixed.

> - Why does "meta" have "src" and "srcs" attributes?

I want to fetch only sources sometimes (as in I will leave soon, I will be offline, but power is not a problem). So I want meta.src or just src to always contain main source download, and meta.srcs would contain additional downloads. I want to mark up new packages first.  So I will once have enough packages to force me to write expressions that actually automate the fetching.

> - For consistency, don't put a "." at the end of the description, and write it 
> on one line, i.e.
> 
>      meta = {
>        description = "A TeX distribution";
>      };
> 
>    (BTW, "description" is intended for one-line descriptions on the Nixpkgs 
> release page, but maybe we could also use an attribute for longer descriptions.)

OK. 

> To reflect on the whole builderDefs / FullDepEntry / textClosure / null 
> arguments style: it looks to me significantly more complicated than the old 
> style of writing Nix expressions, compare e.g. the TeXLive expression to the 
> teTeX expression 

Not honest. teTeX build process is sane and polished. But generally, yes there were too many things filled in from template.

> (https://svn.cs.uu.nl:12443/repos/trace/nixpkgs/trunk/pkgs/misc/tex/tetex/default.nix). 
>   There is a lot of duplicated code between expressions written in the style, 
> which suggests that some abstraction is in order: if you find yourself writing 
> (or cutting&pasting) the same code over and over, then it's time to put the 
> commonality in a function...

OK.. First everything was very very unstable, so I tuned individual instances. Now it seems to stabilize, so you are right and I actually have tried to move everything in a function... If it survives rebuild, I will commit it.