Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test and executable runner options #221

Conversation

everythingfunctional
Copy link
Member

Allow a way of specifying a command to be used to run the executable(s) or test(s).

An example of where this might be particularly useful (practically necessary) is

fpm test --runner "valgrind --leak-check=full --error-exitcode=1"

to run the tests while looking for memory leaks.

@everythingfunctional
Copy link
Member Author

Note, this should be merged after #220

@everythingfunctional
Copy link
Member Author

While #220 makes it effectively impossible to know the path to the produced executable (which is something we were trying to avoid specifying anyway), this, combined with the -- ARGS functionality should remove most users' needs to know it anyway. Whatever command a user would like to run with the executable is now constructable via fpm as [optional runner ]path/to/executable[ optional args] constructed via fpm run [--runner "optional runner"] --target executable [-- optional args]

@urbanjost
Copy link
Contributor

As mentioned the --flag option does not allow multiple compiler options. A small errata is to change addional ==> additional in the help text. Wondering if for a cross-mounted installation if the UUID for the directory should include some kind of system identifier as well; so that if I compile on a big-endian and small-endian machine from the same directory, for example, that I get two different output directories. Did not look carefully at how the key is generated, but right now using the same flags in a different order generates a different output file (ie. fpm run A --flag -Wextra --flag -Wall uses a different output file than fpm run A --flag -Wall --flag Wextra). So one reason not to allow multiple arguments with a single --flag switch might be to more easily identify the flags are the same. One big difference still remaining between the Fortran and bootstrap version is that fpm new A with no explicit switches defaults to no directories for the bootstrap version and being equivalent to fpm new --lib --app --test in the Fortran version. Good with it as an Alpha feature for people to experiment with; but not being able to specify more than one flag at a time becomes very verbose when trying to duplicate the switches used by --release, for example. Assuming the unique key based on options (a clever idea) is not reversible it is somewhat hard after you have generated a bunch of binaries to remember which is which (A profiling version, a version with coarray set to one processor and set to multiple processors, ...) so maybe some kind of labeling mechanism or recording of the options used would seem useful. So good with both #220 and #221 as alpha versions but hoping something emerges to resolve those questions as people try it. A footnote is that to add a similiar option to the Fortran version would either take a restriction that a flag value has to start with a space if the first character would otherwise be a dash or a slight change to the CLI interface would be needed but would be relatively straight-forward. So unless requiring --flag " -option value -option value(s) ..." is too unintuitive
that would be non-trivial to add. Adding something like --flag 'NAME: -opt [value] -opt[value]' would let you provide a tag for the options and prevent the problem with non-numeric values starting with a dash, however.

PS: For testing I found the intrinsics for echoing the compiler version and options useful. Last I checked a lot of compilers still did not implement that; but maybe each build directory could have a little program built into it that called those functions and maybe even uname -a and date information (although doing that portably right now is an issue). Just for reference I like the output to be a little more readable from gfortran so I used to following for some of the testing; albeit I admit to a bit of overkill just to parse on spaces:

program demo_compiler_version
use, intrinsic :: iso_fortran_env, only : compiler_version, compiler_options
implicit none
character(len=:),allocatable :: args(:)
character(len=4096)          :: line
integer                      :: i
write(line,'(a)')compiler_options()
   write (*,'(a)')repeat('=',80)
   print '(a)', &
    'This file was compiled by ', &
    compiler_version(),           &
    'using the options'
   call split(line,args)
   write(*,'(a)')(trim(args(i)),i=1,size(args))
   write (*,'(a)')repeat('=',80)
contains
subroutine split(input_line,array)
character(len=*),intent(in)              :: input_line
character(len=:),allocatable,intent(out) :: array(:)
integer,allocatable         :: ibegin(:), iterm(:)
character(len=*),parameter  :: dlim=' '//char(9)//char(10)//char(11)//char(12)//char(13)//char(0)
integer :: n, ii, icount, ilen, i10, i20, i30, icol, idlim, ifound, inotnull, imax
   idlim=len(dlim)
   n=len(input_line)+1
   allocate(ibegin(n),iterm(n))
   ibegin(:)=1
   iterm(:)=1
   icount=0
   inotnull=0
   imax=0
   ilen=len(input_line)
   if(ilen.gt.0)then
      icol=1
      INFINITE: do i30=1,ilen,1
         ibegin(i30)=icol
         if(index(dlim(1:idlim),input_line(icol:icol)).eq.0)then
            iterm(i30)=ilen
            do i10=1,idlim
               ifound=index(input_line(ibegin(i30):ilen),dlim(i10:i10))
               if(ifound.gt.0) iterm(i30)=min(iterm(i30),ifound+ibegin(i30)-2)
            enddo
            icol=iterm(i30)+2
            inotnull=inotnull+1
         else
            iterm(i30)=icol-1
            icol=icol+1
         endif
         imax=max(imax,iterm(i30)-ibegin(i30)+1)
         icount=i30
         if(icol.gt.ilen) exit INFINITE
      enddo INFINITE
   endif
   allocate(character(len=imax) :: array(inotnull))
   ii=1
   do i20=1,icount
      if(iterm(i20).lt.ibegin(i20))then
      else
         array(ii)=input_line(ibegin(i20):iterm(i20))
         ii=ii+1
      endif
   enddo
end subroutine split
end program demo_compiler_version

@urbanjost
Copy link
Contributor

PS: Was not clear from --target option that "fpm run A B C" still worked or whether you could have multiple names on --target or specify --target more than once. Wondering if with --target you can combine it with command options instead of using -- to something like fpm run --target 'A -x 10.3 -y 20' might work. Was wondering why the change to having an option like --target when the original just asked for a list of executable names.

@urbanjost
Copy link
Contributor

urbanjost commented Oct 31, 2020

I keep running into cases where using xargs(1) makes it easier as not all commands take the filenames at the end.
Since I do not know of an equivalent command to xargs(1) on non-GNU/Unix platforms (is there one?) I am wondering if something like the mask that you can do with xargs(1) would be useful, where if the --runner command has some special string in it like %FILE that gets replaced with the filename? This option allows for testing things that might become options like installing executables in a specified directory, listing app names when you have multiple ones, thinking about how profiling or debuggers might be incorporated, and so on. Would be useful to do on source files as well as executables. Works well in GNU/Unix "toolbox" environment, not sure if it works as well in MSWindows environment.

fpm run --runner 'ls -l'
 fpm run --runner file
 fpm run --runner time
 fpm run --runner sum
 fpm run --runner 'valgrind --leak-check=full -s'
# get list of pathnames
 fpm run --runner 'echo'
# debugger
 fpm run --runner 'gdb'
# combine echo and xargs
 fpm run --runner 'echo'|xargs -iXX cp XX /tmp/bin/
# copy with name at end of command (people usually use "cp FILE DIR/")
 fpm run --runner 'cp --target-directory=/tmp/bin/ --update'
# make nice little table of names that can be run or tested
 fpm test --runner echo|xargs -iXX basename XX|xargs -n 5|column -t
# install binaries in specified directory
fpm run --runner 'env VERSION_CONTROL=numbered install -D --owner=$LOGNAME --preserve-timestamps --backup --suffix=`date +%Y%M%D` --group=$(id -nG) --mode=0711 --target-directory=/tmp/bin/fpm --verbose --preserve-context'

@everythingfunctional
Copy link
Member Author

Yeah, I'm not totally enamored with the verbosity of having to specify every flag with --flag in front of it. I'm not sure exactly what the solution is yet though.

For recording compiler version and options, we could just put a log file at the base of the build directory that identifies them. Just printing the exact strings used to generate the hashes would probably be sufficient for most cases.

fpm run A B C to specify multiple targets never worked, but I could definitely be persuaded to allow for multiple --target.

@everythingfunctional everythingfunctional changed the title WIP: Test and executable runner options Test and executable runner options Nov 12, 2020
@everythingfunctional
Copy link
Member Author

With #220 merged in, this is now ready as well. I don't think I've seen any significant objections, so I'd like to merge it sooner rather than later, but I'll give a day or two for anybody to take another look

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay in reviewing. I've played around with it using valgrind and gdb and all works as expected 👍

@everythingfunctional
Copy link
Member Author

Having seen no objections, and indications that the Fortran version will soon have this same functionality, I'm going to go ahead and merge.

@everythingfunctional everythingfunctional merged commit 213d343 into fortran-lang:master Nov 16, 2020
@everythingfunctional everythingfunctional deleted the test_runner_option branch November 16, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants